Hi again,
Dmitry Ivankov writes:
>>> - if (svndump_init(NULL, url, ref, backflow_fd))
>>> + if (svndump_init(NULL, url, ref, backflow_fd, progress))
>>
>> You're modifying the svndump_init API for every new option that's
>> added. This'll clearly break down when you have many options -- how
>> about wrapping it up in an options structure and then passing that?
> Well, there has to be a function to init that structure then. And the
> structure will become a part of API.
> So don't know if it's worthy.
Well, you'll be wrapping up url, ref, backflow_fd, progress, and all
other future command-line parameters in one variable. Think about it.
You can use the parse-options API to fill in default arguments
anyway.
>>> diff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt
>>> index a7ad368..f1a459e 100644
>>> --- a/contrib/svn-fe/svn-fe.txt
>>> +++ b/contrib/svn-fe/svn-fe.txt
>>> @@ -39,6 +39,9 @@ OPTIONS
>>> Integer number of file descriptor from which
>>> responses to 'ls' and 'cat-blob' requests will come.
>>> Default is fd=3.
>>> +--[no-]progress::
>>> + Write 'progress' lines to fast-import stream. These
>>> + can be displayed by fast-import.
>>
>> Hm, this will make it a little too silent for the end-user. What do
>> you feel about printing something minimalistic like a '.' for each
>> imported revision? Atleast it won't look like it's hung.
> For a medium 8k commit repo it is 100 lines - still too much.
> A single line for the first revision seem harmless and will indicate
> that the remote connection succeeded
> (helps to see that it's not a connection timeout, probably caused by
> dns lookup or a firewall).
Ah, right. I agree with you.
>> Also, how does this interact with the 'progress' option of fast-import protocol?
> git fast-import --quiet prints any progress line produced by a helper.
> transport-helper.c tries to set the option
> but doesn't fail if it is not accepted or if helper doesn't support
> options at all.
> For now the helper doesn't use this protocol option.
>
> A better solution could be to use progress.o api to display progress.
> Or an ad-hoc hack with adaptive progress step, say report a progress
> on each "power of two"-th revision.
> As a starting point for tests let there bust a simple switch-off.
Yeah, that makes sense -- would love to see that in future.
>>> -void fast_export_init(int fd, const char *dst_ref)
>>> +void fast_export_init(int fd, const char *dst_ref, int progress)
>>> {
>>> first_commit_done = 0;
>>> + print_progress = progress;
>>
>> The only reason you're modifying the API of fast_export_init is so
>> that it can set a global static variable?
> Looked once more at how these new variables are used. We can move
> progress lines generation to svndump.c. And also move ref_name from
> _init and being global static to a parameter in fast_export_begin_commit(),
> and to be more sane s/revision/target_mark/ s/revision - 1/from_mark/ and
> add a from_mark parameter, move first_commit_done logic to svndump.o.
> This way fast_export.o can operate on single commits and maybe it'll be
> easier to use it to apply svn branches layout in svn-fe in one run, though I'm
> not sure I'll use svn-fe to manage svn branches.
Makes sense.
Thanks.
-- Ram
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html