Re: [PATCH 1/2] Add git-archive

From: Franck Bui-Huu <vagabon.xyz@gmail.com>
Date: Sep 7, 2006, 06:32 UTC
Message-ID: <cda58cb80609062332p356bd26bw852e31211c43d1ac@mail.gmail.com>
In-Reply-To: <7vac5c7jty.fsf@assigned-by-dhcp.cox.net>

2006/9/6, Junio C Hamano <junkio@cox.net>:

Show 6 quoted lines

> Franck Bui-Huu <vagabon.xyz@gmail.com> writes:
>
> > Interesting, could you explain why static variables are not nice ?
>
> Mostly taste and a little bit of re-entrancy worries.
>

OK.

Show 11 quoted lines

> > You might have missed my second patch:
> >
> >               "[PATCH 2/2] Add git-upload-archive"
> >
> > Basically the server can also use 'reason' to report a failure
> > description during NACK. I find it more useful than the simple
> > "server sent EOF" error message.
>
> That's a good intention, but we would also need to convey the
> "server side found problem and died with these error() output"
> anyway,

OK.

Show 7 quoted lines

>
> > One bad side is that we need to malloc this embedded structure.
>
> Not at all, if you read the example I did you would notice that
> I changed parse_extra for each backend to return this structure
> allocated for that particular backend.
>

sorry I wasn't clear. My point was that the structure need to be 'mallocated'. Which funtion allocate it doesn't matter, we will need to free it later. That's what I tried to avoid with the alternative I sent you in my previous email. Do you think we could use it ?

Show 18 quoted lines

> >>> +static int run_remote_archiver(struct archiver_struct *ar, int argc,
> >>> +                          const char **argv)
> >>> +{
> >>> +   char *url, buf[1024];
> >>> +   pid_t pid;
> >>> +   int fd[2];
> >>> +   int len, rv;
> >>> +
> >>> +   sprintf(buf, "git-upload-%s", ar->name);
> >>
> >> Are you calling git-upload-{tar,zip,rar,...} here?
> >
> > yes. Actually git-upload-{tar,zip,...} commands are going to be
> > removed, but git-daemon know them as a daemon service.
>
> That would break "git-archive --remove=ssh://site/repo treeish"
> wouldn't it?
>

Yes. But couldn't we make some alias like:

alias git-upload-tar='git-upload-archive --format=tar' alias git-upload-zip='git-upload-zip --format=zip'

and the same could be done if we plan to remote git-tar-tree command:

alias git-tar-tree='git-archive --format=tar'

These alias would be internal to git (always defined)

-- 
               Franck