| Message ID | 20190829210301.18467-1-dturner@twosigma.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clarify documentation for remote helpers | expand |
On Thu, 29 Aug 2019 at 23:06, David Turner <dturner@twosigma.com> wrote: > -Optionally may output a 'lock <file>' line indicating a file under > -GIT_DIR/objects/pack which is keeping a pack until refs can be > -suitably updated. > +Optionally may output a 'lock <file>' line indicating the full path of > +a file under under GIT_DIR/objects/pack which is keeping a pack until > +refs can be suitably updated. The path must end with ".keep". "under under". Also -- and I realize this is nothing new in your patch -- "GIT_DIR" should be prefixed with a '$' and that whole path wrapped in backticks so it gets monospaced. In total, my suggestion would be -+a file under under GIT_DIR/objects/pack which is keeping a pack until ++a file under `$GIT_DIR/objects/pack` which is keeping a pack until Whether what you're saying is actually *true*, sorry, no idea. I just have those nits above to offer. Martin
I was confused, because I read "a file under GIT_DIR/objects/pack" to mean "just the filename". Some of the things that deal with packs take just the filename (e.g. --keep-pack for git repack). I'll fix the under under and add $, but I do want to clarify that it's the full path. > -----Original Message----- > From: Martin Ågren <martin.agren@gmail.com> > Sent: Friday, August 30, 2019 12:03 AM > To: David Turner <David.Turner@twosigma.com> > Cc: Git Mailing List <git@vger.kernel.org> > Subject: Re: [PATCH] clarify documentation for remote helpers > > On Thu, 29 Aug 2019 at 23:06, David Turner <dturner@twosigma.com> wrote: > > > -Optionally may output a 'lock <file>' line indicating a file under > > -GIT_DIR/objects/pack which is keeping a pack until refs can be > > -suitably updated. > > +Optionally may output a 'lock <file>' line indicating the full path > > +of a file under under GIT_DIR/objects/pack which is keeping a pack > > +until refs can be suitably updated. The path must end with ".keep". > > "under under". > > Also -- and I realize this is nothing new in your patch -- "GIT_DIR" > should be prefixed with a '$' and that whole path wrapped in backticks so it > gets monospaced. In total, my suggestion would be > > -+a file under under GIT_DIR/objects/pack which is keeping a pack until > ++a file under `$GIT_DIR/objects/pack` which is keeping a pack until > > Whether what you're saying is actually *true*, sorry, no idea. I just have those > nits above to offer. > > Martin
David Turner <David.Turner@twosigma.com> writes: > I was confused, because I read "a file under GIT_DIR/objects/pack" > to mean "just the filename". Some of the things that deal with > packs take just the filename (e.g. --keep-pack for git repack). > I'll fix the under under and add $, but I do want to clarify that > it's the full path. I think that the phrase wanted to say that the file named with the option must live under that directory, without any implication that the directory is used as the base when a relative path is used. If the helper MUST give a full pathname and a pathname relative to that directory is not accepted, it is a good idea to spell it out (also if it must end with ".keep", that also should be documented---is there any other restrictions?).
> -----Original Message----- > From: Junio C Hamano <gitster@pobox.com> > Sent: Friday, August 30, 2019 1:35 PM > To: David Turner <David.Turner@twosigma.com> > Cc: Martin Ågren <martin.agren@gmail.com>; Git Mailing List > <git@vger.kernel.org> > Subject: Re: [PATCH] clarify documentation for remote helpers > > David Turner <David.Turner@twosigma.com> writes: > > > I was confused, because I read "a file under GIT_DIR/objects/pack" > > to mean "just the filename". Some of the things that deal with packs > > take just the filename (e.g. --keep-pack for git repack). > > I'll fix the under under and add $, but I do want to clarify that it's > > the full path. > > I think that the phrase wanted to say that the file named with the option must > live under that directory, without any implication that the directory is used as > the base when a relative path is used. If the helper MUST give a full pathname > and a pathname relative to that directory is not accepted, it is a good idea to > spell it out (also if it must end with ".keep", that also should be documented--- > is there any other restrictions?). The only other restriction I see is: in order for the connectivity-skipping optimization to be used, the file with s/.keep/.idx/, and the corresponding pack, must exist. Do you think that's worth mentioning? It seems to be implied by the rest of the text.
David Turner <David.Turner@twosigma.com> writes: > The only other restriction I see is: in order for the connectivity-skipping > optimization to be used, the file with s/.keep/.idx/, and the corresponding > pack, must exist. Do you think that's worth mentioning? It seems to be > implied by the rest of the text. Perhaps if you come up with a way to clarify what is merely implied (e.g. "this is a mechanism to name a <pack,idx,keep> tuple by giving only the keep component to do X"), I would imaging that would be ideal. Thanks.
diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index 43f80c8068..30001b2054 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -297,9 +297,9 @@ Supported if the helper has the "option" capability. same batch are complete. Only objects which were reported in the output of 'list' with a sha1 may be fetched this way. + -Optionally may output a 'lock <file>' line indicating a file under -GIT_DIR/objects/pack which is keeping a pack until refs can be -suitably updated. +Optionally may output a 'lock <file>' line indicating the full path of +a file under under GIT_DIR/objects/pack which is keeping a pack until +refs can be suitably updated. The path must end with ".keep". + If option 'check-connectivity' is requested, the helper must output 'connectivity-ok' if the clone is self-contained and connected.
Signed-off-by: David Turner <dturner@twosigma.com> --- This doesn't address the connectivity-ok problem, which I continue to worry is a real bug. But it would have saved me a few minutes of debugging. Documentation/gitremote-helpers.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)