On Sat, Jul 19, 2008 at 09:07:55PM +0200, Jakub Narebski wrote:
> On Fri, 18 July 2008, Petr Baudis wrote:
> > On Tue, Jul 15, 2008 at 01:41:38AM +0200, Jakub Narebski wrote:
> > > On Mon, 14 July 2008, Petr Baudis wrote:
> > > > Here is an idea: Introduce Git::Command object that will have very
> > > > general interface and look like
> > > >
> > > > my $c = Git::Command->new(['git', '--git-dir=.', 'cat-file', \
> > > > '-p', 'bla'], {pipe_out=>1})
> > > > ...
> > > > $c->close();
> > >
> > > Errr... how do you read from such a pipe? <$c> I think wouldn't work,
> > > unless you would use some trickery...
> >
> > That's good point; it might either be done using some trickery, or
> > $c->pipe. The idea behind having a special object for it though is to
> > have *unified* (no matter how simple) error handling. You might not
> > detect the command erroring out at the open time.
> >
> > Is there a better approach for solving this?
>
> I don't know if it is _better_ approach, but the _alternate_ approach
> would be to use:
>
> my $c = Git::Command->new(['git', '--git-dir=.', 'cat-file', \
> '-p', 'bla'], {out=>my $fh, err=>undef})
> ...
> while (my $line = <$fh>) {
> ...
> $c->close();
I think this is horribly ugly, you would be *much* better keeping the
filehandle within $c if going this way.
> And trickery would be to use blessed filehandle, or what? Or perhaps
> extending IO::Handle (but not all like using object methods for I/O
> handles)?
Maybe blessed filehandle is the simplest way; it seems that in case we
need anything more complex later, it should be possible to replace it
with an IO::Handle subclass, but that feels like overengineering now.
> I forgot that we cannot obsolete / replace old interface. Nevertheless
> it would be nice to be able to use for example
>
> Git::Cmd->output_pipe('ls-remotes', $URL, '--heads');
>
> but also
>
> output_pipe('myscript.sh', <arg1>, <arg2>);
I think exported functions should have all a git_ prefix.
> > Well, this interface is almost identical to what I delineated, except
> > that I have the extra ->cmd-> step there. But maybe, we could go with
> > your API and instead have Git::CommandFactory as a base of Git::Repo?
> > The hierarchy would be
> >
> > Git::CommandFactory - provides the cmd_pipe toolkit
> > |
> > Git::Repo - provides repository model
> > |
> > Git::Repo::NonBare - additional working-copy-related methods
> >
> > I think I will post a sample implementation sometime over the weekend.
>
> Thanks.
>
> I think this is a very good idea. Although... you mix somewhat here
> relationships. Relationship between Git::CommandFactory (Git::Cmd?)
> is a bit different than relationship between Git::Repo and
> Git::Repo::NonBare. Git::Repo::NonBare is a case of Git::Repo which
> additionally knows where its working copy (Git::WC?) is, and where
> inside working copy we are (if we are inside working copy). Git::Repo
> uses Git::CommandFactory to route calls to git commands, and to
> provide default '--git-dir=<repo_path>' argument.
Yes, but that does not mean Git::Repo must not inherit from
Git::CmdFactory. Think of Git::CmdFactory as maybe a kind of Java-sense
interface to a degree.
> What I'd like to have is a way to easily set in _one_ place where git
> binary can be found, even if we are using different repositories, call
> git commands not related to git repository.
>
> Should we use
>
> Git::Cmd->output_pipe('ls-remotes', $URL, '--heads');
> or
> output_pipe(GIT, 'ls-remotes', $URL, '--heads');
> or
> output_pipe($GIT, 'ls-remotes', $URL, '--heads');
> or
> output_pipe($Git::GIT, 'ls-remotes', $URL, '--heads');
>
> we would want to be able to set where git binary is once (and for all),
> for example via
>
> Git::Cmd->set_git('/usr/local/bin/git');
>
> or something like that.
Yes, that should work fine, with the Git::Cmd subclasses looking into
the singleton.
BTW, I don't like Git::Cmd for the factory interface, since the methods
create Git::Command objects and then the naming does not make any sense.
So I'm going to use class names Git::CmdFactory and Git::Cmd for the
first prototype (since "Command" _is_ too long), unless you have better
but still clear names.
--
Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name. -- Ken Thompson and Dennis M. Ritchie
--
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
