git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH v3] upload-pack: send shallow info over stdin to pack-objects
Date: Sat, 8 Mar 2014 07:08:05 +0700	[thread overview]
Message-ID: <CACsJy8AcYBXi6LjyJDeEnogPTXfqYXqijXaLY=bUgNnd4cT_Fg@mail.gmail.com> (raw)
In-Reply-To: <xmqq8uslvp86.fsf@gitster.dls.corp.google.com>

On Sat, Mar 8, 2014 at 1:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> On the receive-pack side, the comment at the bottom of
>>> preprare_shallow_update() makes it clear that, if we wanted to use
>>> hooks, we cannot avoid having the proposed new shallow-file in a
>>> temporary file, which is unfortunate.  Do we have a similar issue on
>>> hooks on the upload-pack side?
>>
>> No. I don't think we have hooks on upload-pack.
>
> The question was not only about "do we happen to work OK with the
> current code?" but about "are we closing the door for the future?"
>
> If we ever need to add hooks to upload-pack, with the updated
> approach, its operation will not be affected by the temporary
> shallow file tailored for this specific customer.  Which I think is
> a good thing in general.
>
> But at the same time, it means that its operation cannot be
> customized for the specific customer, taking into account what they
> lack (which can be gleaned by looking at the temporary shallow
> information).  I do think that it is not a big downside, but that is
> merely my knee-jerk reaction.

When upload-pack learns about hooks, I think we'll need to go back
with --shallow-file, perhaps we a secure temporary place to write in.
I don't see another way out. Not really sure why upload-pack needs
customization though. The only case I can think of is to prevent most
users from fetching certain objects, but that does not sound
realistic..

>>> Nothing for Documentation/ anywhere?
>>
>> Heh git-pack-objects.txt never described stdin format. At least I
>> searched for --not in it and found none. So I gladly accepted the
>> situation and skipped doc update :D
>
> To pile new technical debt on top of existing ones is to make things
> worse, which we would rather not to see.

Of course.. So what should we do with this? Go with this "no temp
file" approach and deal with hooks when they come, or prepare now and
introduce a secure temp. area?
-- 
Duy

  reply	other threads:[~2014-03-08  0:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-27  7:13 [PATCH] upload-pack: allow shallow fetching from a read-only repository Nguyễn Thái Ngọc Duy
2014-02-27  9:04 ` Jeff King
2014-02-27  9:10   ` [PATCH] shallow: verify shallow file after taking lock Jeff King
2014-02-27  9:22     ` Jeff King
2014-02-27 10:18       ` Duy Nguyen
2014-02-27 10:56         ` [PATCH] shallow: use stat_validity to check for up-to-date file Jeff King
2014-02-27 10:11   ` [PATCH] upload-pack: allow shallow fetching from a read-only repository Duy Nguyen
2014-02-27 11:25     ` [PATCH] shallow: automatically clean up shallow tempfiles Jeff King
2014-03-04 12:30 ` [PATCH v2] upload-pack: allow shallow fetching from a read-only repository Nguyễn Thái Ngọc Duy
2014-03-04 18:10   ` Junio C Hamano
2014-03-05 12:43     ` Duy Nguyen
2014-03-05 19:50       ` Junio C Hamano
2014-03-06  8:49   ` [PATCH v3] upload-pack: send shallow info over stdin to pack-objects Nguyễn Thái Ngọc Duy
2014-03-06 18:37     ` Junio C Hamano
2014-03-06 23:13       ` Duy Nguyen
2014-03-07 18:27         ` Junio C Hamano
2014-03-08  0:08           ` Duy Nguyen [this message]
2014-03-10 15:23             ` Junio C Hamano
2014-03-07  1:24     ` Duy Nguyen
2014-03-07 18:33       ` Junio C Hamano
2014-03-11 12:59     ` [PATCH v4] " Nguyễn Thái Ngọc Duy

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACsJy8AcYBXi6LjyJDeEnogPTXfqYXqijXaLY=bUgNnd4cT_Fg@mail.gmail.com' \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).