From: Petr Baudis <pasky@suse•cz>
To: Jakub Narebski <jnareb@gmail•com>
Cc: Lea Wiemann <lewiemann@gmail•com>,
git@vger•kernel.org, John Hawley <warthog19@eaglescrag•net>
Subject: Re: [PATCH 2/3] add new Git::Repo API
Date: Sun, 20 Jul 2008 23:36:42 +0200 [thread overview]
Message-ID: <20080720213642.GE10151@machine.or.cz> (raw)
In-Reply-To: <200807192107.56333.jnareb@gmail.com>
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
next prev parent reply other threads:[~2008-07-20 21:37 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-11 1:06 [PATCH 0/3] Git::Repo API and gitweb caching Lea Wiemann
2008-07-11 1:10 ` [PATCH 1/3 v9] gitweb: add test suite with Test::WWW::Mechanize::CGI Lea Wiemann
2008-07-11 1:11 ` [PATCH 2/3] add new Git::Repo API Lea Wiemann
2008-07-13 21:38 ` Junio C Hamano
2008-07-14 1:04 ` Lea Wiemann
2008-07-13 23:28 ` Jakub Narebski
2008-07-14 2:29 ` Lea Wiemann
2008-07-14 1:40 ` Petr Baudis
2008-07-14 22:19 ` Lea Wiemann
2008-07-18 16:48 ` Petr Baudis
2008-07-18 17:05 ` Jakub Narebski
2008-07-18 17:17 ` Petr Baudis
2008-07-18 18:09 ` Lea Wiemann
2008-07-18 18:19 ` Petr Baudis
2008-07-18 18:23 ` Johannes Schindelin
2008-07-19 20:54 ` Statictics on Git.pm usage in git commands (was: [PATCH 2/3] add new Git::Repo API) Jakub Narebski
2008-07-19 21:14 ` Petr Baudis
2008-07-20 0:16 ` Jakub Narebski
2008-07-20 21:38 ` Petr Baudis
2008-07-20 10:38 ` Johannes Schindelin
2008-07-20 10:49 ` Petr Baudis
2008-07-20 12:33 ` Johannes Schindelin
2008-07-20 12:58 ` Petr Baudis
2008-07-20 13:21 ` Johannes Schindelin
2008-07-14 23:41 ` [PATCH 2/3] add new Git::Repo API Jakub Narebski
2008-07-15 0:11 ` Lea Wiemann
2008-07-18 16:54 ` Petr Baudis
2008-07-19 0:03 ` Jakub Narebski
2008-07-19 19:07 ` Jakub Narebski
2008-07-20 21:36 ` Petr Baudis [this message]
2008-07-20 21:50 ` Jakub Narebski
2008-07-16 18:21 ` Jakub Narebski
2008-07-16 20:32 ` Lea Wiemann
2008-07-17 23:49 ` Jakub Narebski
2008-07-18 13:40 ` Lea Wiemann
2008-07-18 15:35 ` Jakub Narebski
2008-07-18 16:51 ` Lea Wiemann
2008-07-11 1:11 ` [PATCH 3/3] gitweb: use new Git::Repo API, and add optional caching Lea Wiemann
2008-07-14 21:23 ` Jakub Narebski
2008-07-14 23:03 ` Lea Wiemann
2008-07-14 23:14 ` Jakub Narebski
2008-07-14 23:56 ` Lea Wiemann
2008-07-15 0:52 ` Jakub Narebski
2008-07-15 1:16 ` Lea Wiemann
2008-07-15 1:28 ` Johannes Schindelin
2008-07-15 1:44 ` J.H.
2008-07-15 1:50 ` Lea Wiemann
2008-07-15 2:03 ` J.H.
2008-07-11 1:21 ` [PATCH 0/3] Git::Repo API and gitweb caching Johannes Schindelin
2008-07-11 9:33 ` Jakub Narebski
2008-07-11 14:07 ` Lea Wiemann
2008-07-11 16:27 ` Abhijit Menon-Sen
2008-07-12 15:08 ` Jakub Narebski
2008-07-19 5:35 ` Lea Wiemann
2008-08-18 19:34 ` Lea Wiemann
2008-08-18 19:39 ` [PATCH 1/3 v10] gitweb: add test suite with Test::WWW::Mechanize::CGI Lea Wiemann
2008-08-19 1:17 ` Junio C Hamano
2008-08-19 14:37 ` Lea Wiemann
2008-08-18 19:39 ` [PATCH 2/3 v2] add new Perl API: Git::Repo, Git::Commit, Git::Tag, and Git::RepoRoot Lea Wiemann
2008-08-19 1:32 ` Junio C Hamano
2008-08-19 15:06 ` Lea Wiemann
2008-08-19 13:51 ` Lea Wiemann
2008-08-18 19:39 ` [PATCH 3/3 v2] gitweb: use new Git::Repo API, and add optional caching Lea Wiemann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080720213642.GE10151@machine.or.cz \
--to=pasky@suse$(echo .)cz \
--cc=git@vger$(echo .)kernel.org \
--cc=jnareb@gmail$(echo .)com \
--cc=lewiemann@gmail$(echo .)com \
--cc=warthog19@eaglescrag$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox