threads / patch / 13123

patchgit-fast-import: note 1M limit of mark number

Subject: [PATCH] git-fast-import: note 1M limit of mark number

## tl;dr

5 messages between Apr 15, 2008 and Apr 16, 2008. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Sam Vilain· Apr 15, 2008, 12:54 UTC · lore

The insert_mark() function in fast-import.c has this limit; note the limitation in the documentation.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
 Documentation/git-fast-import.txt |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to Documentation/git-fast-import.txt +3 −0
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c29a4f8..b5cc3c2 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -410,6 +410,9 @@ or `refs/heads/42`), or an abbreviated SHA-1 which happened to
 consist only of base-10 digits.
 +
 Marks must be declared (via `mark`) before they can be used.
++
+Note that due to current internal limitations, you may not make marks
+with a higher number than 1048575 (2^20-1).
 
 * A complete 40 byte or abbreviated commit SHA-1 in hex.
 

-- 
1.5.4.rc2.85.g7c8f5
Michael Haggerty· Apr 15, 2008, 15:50 UTC · re: Sam Vilain · lore

Re: [PATCH] git-fast-import: note 1M limit of mark number

Sam Vilain wrote:
Show 22 quoted lines
> The insert_mark() function in fast-import.c has this limit; note the
> limitation in the documentation.
> 
> Signed-off-by: Sam Vilain <sam@vilain.net>
> ---
>  Documentation/git-fast-import.txt |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index c29a4f8..b5cc3c2 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -410,6 +410,9 @@ or `refs/heads/42`), or an abbreviated SHA-1 which happened to
>  consist only of base-10 digits.
>  +
>  Marks must be declared (via `mark`) before they can be used.
> ++
> +Note that due to current internal limitations, you may not make marks
> +with a higher number than 1048575 (2^20-1).
>  
>  * A complete 40 byte or abbreviated commit SHA-1 in hex.
>  
Oh.  Um.  That is an awkwardly small number nowadays.

cvs2svn has been used for repositories with O(2^20) distinct file revisions (KDE, Mozilla, NetBSD, ...). So this limit will likely be too small for some users.

Moreover, cvs2git needs to generate marks for both file contents and for commits. It generates the latter by adding 1000000000 to the small integer IDs that it uses internally. If git-fast-import only allows 20-bit integers, this makes me wonder why this hasn't broken dramatically in the past. Pure numerological good fortune, combined with weak range checking in git-fast-import?

In any case, this restriction will require changes in cvs2git.

While I'm at it, let me also renew my suggestion that git-fast-import use separate namespaces ("markspaces", so to speak) for file content marks and for commit marks. There is no reason for these distinct types of marks to be located in a shared space of integers.

Michael
Sam Vilain· Apr 15, 2008, 21:05 UTC · re: Michael Haggerty · lore

Re: [PATCH] git-fast-import: note 1M limit of mark number

Michael Haggerty wrote:
Show 12 quoted lines
>> ++
>> +Note that due to current internal limitations, you may not make marks
>> +with a higher number than 1048575 (2^20-1).
>>  
>>  * A complete 40 byte or abbreviated commit SHA-1 in hex.
>>  
> 
> Oh.  Um.  That is an awkwardly small number nowadays.
> 
> cvs2svn has been used for repositories with O(2^20) distinct file
> revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too
> small for some users.

Right. But, if you're not making the importer you write for a conversion of that size restartable, you're insane. So, marking more than 1Mi *marks* in a single gfi session might not be so vital.

It only tripped me up because I was using a database sequence to generate the marks, which meant I hit the ceiling.

Show 6 quoted lines
> Moreover, cvs2git needs to generate marks for both file contents and for
> commits.  It generates the latter by adding 1000000000 to the small
> integer IDs that it uses internally.  If git-fast-import only allows
> 20-bit integers, this makes me wonder why this hasn't broken
> dramatically in the past.  Pure numerological good fortune, combined
> with weak range checking in git-fast-import?
Perhaps.  All I saw was that after I hit 1Mi for the mark ID, the mark
numbers in the returned file were drastically different from the ones I
put in.  I had a glance over this code and it seemed likely to be a
culprit - this docpatch is really more raising awareness of the problem.
 Obviously finding the fault and fixing it would be preferable.
> While I'm at it, let me also renew my suggestion that git-fast-import
> use separate namespaces ("markspaces", so to speak) for file content
> marks and for commit marks.  There is no reason for these distinct types
> of marks to be located in a shared space of integers.

There is a reason, it's because they're both just object IDs. Is it really that much of a drag? I know what you mean though, it meant for my code I had to keep track of which type each mark was.

Sam.
Shawn O. Pearce· Apr 16, 2008, 06:54 UTC · re: Sam Vilain · lore

Re: [PATCH] git-fast-import: note 1M limit of mark number

Sam Vilain <sam@vilain.net> wrote:
Show 19 quoted lines
> Michael Haggerty wrote:
> >> ++
> >> +Note that due to current internal limitations, you may not make marks
> >> +with a higher number than 1048575 (2^20-1).
> >>  
> >>  * A complete 40 byte or abbreviated commit SHA-1 in hex.
> > 
> > Oh.  Um.  That is an awkwardly small number nowadays.
> > 
> > cvs2svn has been used for repositories with O(2^20) distinct file
> > revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too
> > small for some users.
> 
> Right.  But, if you're not making the importer you write for a
> conversion of that size restartable, you're insane.  So, marking more
> than 1Mi *marks* in a single gfi session might not be so vital.
> 
> It only tripped me up because I was using a database sequence to
> generate the marks, which meant I hit the ceiling.

Uhm. Wow. gfi has a bug then; mark numbers are supposed to be whatever uintmax_t is on your platform; for any systems that support a 64 bit off_t (and most do these days) that should be a 64 bit integer value, which we all know easily exceeds 2^20-1.

I'd rather see the bug fixed than a documentation patch.  But I'm
too whacked out from heavy travel in the past two days flying
coast-to-coast and back to attempt debugging gfi and writing such
a fix patch tonight.  Maybe someone else will find it before I
can look at it later.  ;-)
Show 8 quoted lines
> > While I'm at it, let me also renew my suggestion that git-fast-import
> > use separate namespaces ("markspaces", so to speak) for file content
> > marks and for commit marks.  There is no reason for these distinct types
> > of marks to be located in a shared space of integers.
> 
> There is a reason, it's because they're both just object IDs.  Is it
> really that much of a drag?  I know what you mean though, it meant for
> my code I had to keep track of which type each mark was.

Yea. I think I had pointed out the same point to Michael earlier when he asked about it. I didn't want create two different tables of marks because then we either need to extend the language to say which table, or we have to infer it based on position. But either way its just object IDs.

-- 
Shawn.
Michael Haggerty· Apr 16, 2008, 07:04 UTC · re: Sam Vilain · lore

Re: [PATCH] git-fast-import: note 1M limit of mark number

Sam Vilain wrote:
Show 16 quoted lines
> Michael Haggerty wrote:
>>> ++
>>> +Note that due to current internal limitations, you may not make marks
>>> +with a higher number than 1048575 (2^20-1).
>>>  
>>>  * A complete 40 byte or abbreviated commit SHA-1 in hex.
>>>  
>> Oh.  Um.  That is an awkwardly small number nowadays.
>>
>> cvs2svn has been used for repositories with O(2^20) distinct file
>> revisions (KDE, Mozilla, NetBSD, ...).  So this limit will likely be too
>> small for some users.
> 
> Right.  But, if you're not making the importer you write for a
> conversion of that size restartable, you're insane.  So, marking more
> than 1Mi *marks* in a single gfi session might not be so vital.

Well, call me insane then. Because there *are* no bulletproof incremental CVS importers, and it would be a lot of work to write one. (git-cvsimport doesn't count because it is far from bulletproof.) So it is reasonable to choose a high-quality one-time conversion over a broken restartable conversion.

Show 8 quoted lines
>> While I'm at it, let me also renew my suggestion that git-fast-import
>> use separate namespaces ("markspaces", so to speak) for file content
>> marks and for commit marks.  There is no reason for these distinct types
>> of marks to be located in a shared space of integers.
> 
> There is a reason, it's because they're both just object IDs.  Is it
> really that much of a drag?  I know what you mean though, it meant for
> my code I had to keep track of which type each mark was.

That's exactly my point. That's at least two of us then who have had extra work to merge two separate ID concepts into a single integer sequence. It's not a big deal, but it would be convenient if git-fast-import allowed the sequences to overlap.

Michael

← back to recent threads