* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 10:13 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911050141t751d45a0r4e340fa0d10af366@mail.gmail.com>
Marco Costalba wrote:
>
>> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
>> revision
>>
>>
>
> Function:
>
> uint qHash(const QByteArray&);
>
> is already defined in the Qt Core libraries, so I have a link error
> with your patch.
>
By the way, this function of yours is not used anywhere AFAICS.
Abdel.
^ permalink raw reply
* Re: [QGIT PATCH/RFC]
From: Abdelrazak Younes @ 2009-11-05 9:50 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550911050141t751d45a0r4e340fa0d10af366@mail.gmail.com>
Marco Costalba wrote:
> Hi Abdel,
>
> On Wed, Nov 4, 2009 at 15:56, Abdelrazak Younes <younes@lyx.org> wrote:
>
>> Hello Marco,
>>
>> While recompiling latest qgit4, I stumbled accross this. I am not quite sure
>> you used a QLatin1String instead of a QByteArray but the attached seems to
>> work fine...
>>
>>
>
> Unfortunatly I cannot say the same here ;-)
>
>
>
>> -class ShaString;
>> +typedef QByteArray ShaString;
>>
>
> ...... cut ......
>
>
>> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
>> revision
>>
>>
>
> Function:
>
> uint qHash(const QByteArray&);
>
> is already defined in the Qt Core libraries, so I have a link error
> with your patch.
>
Weird... it links just fine here... anyway this can be solved by
renaming your version. Or just using the Qt version if that does the
same thing ;-)
> BTW I don't think I have understood the reason of your patch. Do you
> have a compile error or something ?
>
No, I had some warnings so I looked at the code and I just thought that
QLatin1String was not appropriate here and overkill. And QByteArray
should be faster...
Anyway, this was just FYI, I don't think this patch is important at all :-)
Abdel.
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Erik Faye-Lund @ 2009-11-05 9:41 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: Nicolas Pitre, git, Johannes Sixt
In-Reply-To: <16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com>
On Thu, Nov 5, 2009 at 10:00 AM, Andrzej K. Haczewski
<ahaczewski@gmail.com> wrote:
>
> That way we don't need allocations to simulate pthread init/join API
Yay! By the way, I love the work you're doing here. Getting threaded
delta-searching on Windows is something I'm looking forward to :)
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [QGIT PATCH/RFC]
From: Marco Costalba @ 2009-11-05 9:41 UTC (permalink / raw)
To: Abdelrazak Younes; +Cc: git
In-Reply-To: <4AF19630.2070402@lyx.org>
Hi Abdel,
On Wed, Nov 4, 2009 at 15:56, Abdelrazak Younes <younes@lyx.org> wrote:
> Hello Marco,
>
> While recompiling latest qgit4, I stumbled accross this. I am not quite sure
> you used a QLatin1String instead of a QByteArray but the attached seems to
> work fine...
>
Unfortunatly I cannot say the same here ;-)
>-class ShaString;
>+typedef QByteArray ShaString;
...... cut ......
>
> uint qHash(const ShaString& s) { // fast path, called 6-7 times per
> revision
>
Function:
uint qHash(const QByteArray&);
is already defined in the Qt Core libraries, so I have a link error
with your patch.
BTW I don't think I have understood the reason of your patch. Do you
have a compile error or something ?
Thanks
Marco
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 9:00 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: kusmabite, git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911042111270.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
> What about:
>
> typedef struct {
> HANDLE handle;
> void *(*start_routine)(void *);
> void *arg;
> } pthread_t;
>
> DWORD __stdcall windows_thread_start(LPVOID _self)
> {
> pthread_t *self = _self;
> void *ret = self->start_routine(self->arg);
> return (DWORD)ret;
> }
>
> static inline int pthread_create(pthread_t *thread, const void *unused,
> void *(*start_routine)(void *), void *arg)
> {
> thread->handle = CreateThread(NULL, 0, windows_thread_start,
> thread, 0, NULL);
> [...]
> }
The problem I see is not with pthread_init, but pthread_join. Here's
how it looks:
int pthread_join(pthread_t thread, void **value_ptr);
If pthread_t would be a struct, then we can't call pthread_join like
that... At least that's what I though yesterday, but maybe it can be
done like this:
int win32_pthread_join(pthread_t *thread, void **value_ptr)
{
[...]
}
#define pthread_join(a, b) win32_pthread_join(&(a), (b))
That way we don't need allocations to simulate pthread init/join API
> And thread creation is a relatively rare event compared to e.g. mutex
> lock/unlock, so the indirection shouldn't be noticeable. For the same
> reason, I also think that you could make pthread_create() and
> pthread_join() into a C file instead of being inlined which would reduce
> the code footprint at every call site, and allow for only one instance
> of windows_thread_start() which could then be made static.
Yeah, I'll factor that out to separate file.
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 8:51 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911041915120.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> Careful. At the beginning of the function you'll find:
>
> if (delta_search_threads <= 1) {
> find_deltas(list, &list_size, window, depth, processed);
> return;
> }
>
> That is, if we have thread support compiled in but we're told to use
> only one thread, then the bulk of the work splitting is bypassed
> entirely. Inside find_deltas() there will still be pthread_mutex_lock()
> and pthread_mutex_unlock() calls even if no threads are spawned.
Ah, I wasn't aware of that. Actually why would find_deltas lock if no
threads are used? Maybe, for non-threaded call to find_deltas, locking
could be factored out?
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-05 8:45 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911042039200.10340@xanadu.home>
2009/11/5 Nicolas Pitre <nico@fluxnic.net>:
> Please use die("CreateSemaphore() failed") in the failure case instead
> of returning success.
Sure, will do.
> However, my pthread_cond_init man page says:
And that is weird, because mine man page says:
[[[
pthread_cond_init, pthread_cond_signal, pthread_cond_broadcast, and
pthread_cond_wait never return an error code.
]]]
--
Andrzej
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #01; Wed, 04)
From: Johannes Sixt @ 2009-11-05 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <7vmy31a5p6.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> * rj/maint-simplify-cygwin-makefile (2009-10-27) 1 commit.
> - Makefile: merge two Cygwin configuration sections into one
>
> This is one of the most obviously correct bit from "Compiling on Cygwin
> using MSVC fails" topic I didn't really look at. If J6t is Ok with the
> series, I don't mind queueing the whole thing myself.
I'm actually expecting a resend of the remaining patches in the series
with updated commit messages and with the original patch 3/4 being first.
-- Hannes
^ permalink raw reply
* Re: Problem signing a tag
From: Joshua J. Kugler @ 2009-11-05 8:37 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Alex Riesen, git
In-Reply-To: <4AF28CE4.5000906@drmicha.warpmail.net>
On Wednesday 04 November 2009, Michael J Gruber said something like:
> > So, the docs consider 2 a fatal error, even though it appears it
> > isn't. It seems that
> > http://github.com/git/git/blob/a6dbf8814f433a7fbfa9cde6333c98019f6d
> >b1e4/builtin-tag.c#L202 needs to be patched to something along the
> > lines of:
> >
> > rv = finish_command(&gpg)
> > if ((rv && rv !=2) || !len || len < 0)
> >
> > Probably digging in to the gpg source code to figure out what
> > errors are and aren't fatal would be in order.
> >
> > Thanks again for your help! Glad to know what I needed to do to
> > sign my tags!
>
> Dig dig dig... gpg exits with 2 in a lot of cases, one would need to
> parse fd-error to find out more. But it also looks as if gpg exits
> normally with a good passphrase. So I tried, and at least with gpg
> 1.4.9 and git 1.6.5.2 I can sign tags with "use-agent" and without a
> running agent: I get asked for the passphrase (after reporting the
> agent MIA), and everything's fine.
>
> My gpg returns 0 in this case; it returns 2 only if I don't enter the
> passphrase. So, this seems to depend on the version of gpg. Or on
> entering the correct passphrase ;)
>
> Michael
That is weird. Because when working from the prompt (with agent MIA),
gpg 1.4.9, it would accept my pass phrase, and would print the
signature (either binary or ascii armored), but it will still exit with
2. I don't understand it. I'll pop on #gnupg tomorrow and ask about
it.
Thanks again for your help with this!
j
--
Joshua Kugler
Part-Time System Admin/Programmer
http://www.eeinternet.com
PGP Key: http://pgp.mit.edu/ ID 0x14EA086E
^ permalink raw reply
* Re: Problem signing a tag
From: Michael J Gruber @ 2009-11-05 8:29 UTC (permalink / raw)
To: Joshua J. Kugler; +Cc: Michael J Gruber, Alex Riesen, git
In-Reply-To: <200911040947.50226.joshua@eeinternet.com>
Joshua J. Kugler venit, vidit, dixit 04.11.2009 19:47:
> On Wednesday 04 November 2009, Michael J Gruber said something like:
>>> gpg: problem with the agent - disabling agent use
>>> error: gpg failed to sign the tag
>>> error: unable to sign the tag
>>> $ echo $?
>>> 128
>>>
>>> And when I sign at the prompt:
>>>
>>> $ gpg -sa
>>>
>>> You need a passphrase to unlock the secret key for
>>> user: "Joshua J. Kugler <joshua@azariah.com>"
>>> 1024-bit DSA key, ID 14EA086E, created 2009-08-09
>>>
>>> gpg: problem with the agent - disabling agent use
>>> Blah blah blah blah
>>> Blah blah blah blah
>>> $ echo $?
>>> 2
>>
>> [...]
>>
>> I assume you don't want to use gpg-agent, that should be the easy way
>> out.
>
> Well, I could, but I just haven't set it up. :)
>
>> If that helps you can put "--no-use-agent" in your gpg config.
>
> I commented out use-agent in the config. That worked. THANKS!
>
>> 2 is a non-fatal error, 128 a fatal one, btw.
>
> Well, the 2 was from running gpg alone, and 128 was from git erroring
> out. According to the gpg docs:
>
> "The program returns 0 if everything was fine, 1 if at least a signature
> was bad, and other error codes for fatal errors."
>
> So, the docs consider 2 a fatal error, even though it appears it isn't.
> It seems that
> http://github.com/git/git/blob/a6dbf8814f433a7fbfa9cde6333c98019f6db1e4/builtin-tag.c#L202
> needs to be patched to something along the lines of:
>
> rv = finish_command(&gpg)
> if ((rv && rv !=2) || !len || len < 0)
>
> Probably digging in to the gpg source code to figure out what errors are
> and aren't fatal would be in order.
>
> Thanks again for your help! Glad to know what I needed to do to sign my
> tags!
Dig dig dig... gpg exits with 2 in a lot of cases, one would need to
parse fd-error to find out more. But it also looks as if gpg exits
normally with a good passphrase. So I tried, and at least with gpg 1.4.9
and git 1.6.5.2 I can sign tags with "use-agent" and without a running
agent: I get asked for the passphrase (after reporting the agent MIA),
and everything's fine.
My gpg returns 0 in this case; it returns 2 only if I don't enter the
passphrase. So, this seems to depend on the version of gpg. Or on
entering the correct passphrase ;)
Michael
^ permalink raw reply
* Re: Automatically remote prune
From: John Tapsell @ 2009-11-05 8:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7viqdpemki.fsf@alter.siamese.dyndns.org>
2009/11/5 Junio C Hamano <gitster@pobox.com>:
> John Tapsell <johnflux@gmail.com> writes:
> "what the benefits are to give this information _in the 'branch' output_"
> was what I meant. From the part you omitted from my message:
I omitted it just because, imho, it's not what I 'care about'. I'm
not trying to help advanced users (Users that _want_ to keep
remotes/origin/* clean and users that _want_ to be careful to not lose
commits are both advanced users, imho). I'm just interested in
reducing confusion for non-advanced users. So either not-showing
removed remote branches by default, or showing them but marking them
as deleted.
> A better approach to please the first class of audience may be to
> introduce an option that tells fetch to cull tracking refs that are stale.
> Then "branch -r" output will not show stale refs and there is no place
> (nor need) to show [Deleted] labels.
If it's a non-default option, then it won't help the non-advanced users.
> Such an option won't be very useful for the second class of audience,
> though. For them we would need something else, and it would likely be an
> enhancement to "git remote".
Which still leaves confusion when viewing "git branch -r" since they
would show up there still.
John
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Johannes Sixt @ 2009-11-05 7:55 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1E1FD.1050102@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Junio C Hamano wrote:
>> Shouldn't this be solved by teaching the Makefile about this new "Cygwin
>> but using MSVC as compiler toolchain" combination?
>
> Yes. Err... see patch #3 :-P
A clarifiction: Junio talks about using the MSVC tools to build Cygwin
programs, that is, to merely substitute Cygwin's gcc by MSVC, but to still
link against cygwin's C runtime.
But this is not what this series is about.
When the "MSVC build" of git is made, then the MSVC compiler is used, and
this will always use Microsoft libraries in the resulting executables,
regardless of whether "make MSVC=1" was called from a "cygwin environment"
or from and "msys environment".
This series is about fixing "make MSVC=1" when it is run from a "cygwin
environment" by disentangling the MSVC and Cygwin configuration sections
in the Makefile.
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 13/13] Add Python support library for remote helpers
From: Johan Herland @ 2009-11-05 7:55 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar
In-Reply-To: <1257364098-1685-14-git-send-email-srabbelier@gmail.com>
On Wednesday 04 November 2009, Sverre Rabbelier wrote:
> This patch introduces parts of a Python package called
> "git_remote_helpers" containing the building blocks for
> python helper.
> The actual remote helpers itself are NOT part of this patch.
>
> The patch includes the necessary Makefile additions to build and install
> the git_remote_cvs Python package along with the rest of Git.
There are a few typos in the above. How about:
This patch introduces parts of a Python package called
"git_remote_helpers" containing the building blocks for
remote helpers written in Python.
No actual remote helpers are part of this patch, the patch
only includes the common basics needed to start writing such
helpers.
> The patch includes the necessary Makefile additions to build and install
> the git_remote_cvs Python package along with the rest of Git.
s/git_remote_cvs/git_remote_helpers/
> This patch is based on Johan Herland's git_remote_cvs patch and
> has been improved by the following contributions:
> - David Aguilar: Lots of Python coding style fixes
> - David Aguilar: DESTDIR support in Makefile
>
> Cc: David Aguilar <davvid@gmail.com>
> Cc: Johan Herland <johan@herland.net>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
Otherwise it all looks good. Nice work!
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Jeff King @ 2009-11-05 7:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, Tim Mazid, git
In-Reply-To: <7vhbtai2uy.fsf@alter.siamese.dyndns.org>
On Wed, Nov 04, 2009 at 10:03:49AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Wed, Nov 04, 2009 at 07:41:28AM +0100, Sverre Rabbelier wrote:
> >
> >> On Wed, Nov 4, 2009 at 07:17, Tim Mazid <timmazid@hotmail.com> wrote:
> >> > So instead of invoking 'git checkout REMOTE/BRANCH', do 'git checkout -b
> >> > BRANCH REMOTE/BRANCH'.
> >>
> >> Automagically doing 'git checkout -t remote/branch' when asked to do
> >> 'git checkout remote/branch' was suggested earlier on the list and I
> >> think there was even a patch that implemented it, not sure what the
> >> outcome of the series was. I do remember that Peff was annoyed by it
> >> at the GitTogether though so it might be a bad idea.
> >
> > It's in 'next' now.
>
> Isn't it quite different? What's in 'next' for 1.7.0 is to guess the
> user's intention when:
Sorry, yes, I just saw Sverre's comment and misread the original
proposal. Checking out "$remote/$branch" will still detach the HEAD,
and I don't think anybody has a previous proposal to change that.
> I think this is primarily because the way this DWIM is totally silent in
> the transcript is misleading. If you explain it the way I outlined above,
> I do not think there is any confusion. That is, there is no way for the
> user to get confused if the command sequence were like so:
>
> $ git branch -t foo origin/foo
> Branch foo set up to track remote branch foo from origin.
> $ git checkout foo
> Switched to a new branch 'foo'
>
> ... time passes ...
>
> $ git checkout foo
> Switched to branch 'foo'
> Your branch is behind 'origin/foo' by 1 commit, and can be fast-forwarded.
>
> It could just be a matter of telling what we are doing a bit more
> explicitly when this DWIM kicks in. How about this?
>
> $ git checkout foo
> (first forking your own 'foo' from 'origin/foo')
> Branch foo set up to track remote branch foo from origin.
> Switched to a new branch 'foo'
This is much better than the current behavior, IMHO. It at least says
what is going on, so a user who actually reads the message will have a
chance of knowing what happened.
The devil's advocate argument is that the difference between the "branch
-t" and the DWIM is that in the former, the user intentionally asks for
a new branch, whereas in the latter, they must realize (by reading and
understanding) that a new branch has been created.
Maybe that difference isn't relevant, and people actually read and
understand everything git says. Maybe not. I dunno. I don't think we
have any real data yet on how people will perceive the feature over
time, and I suspect the only way to get it is to release with it and see
what happens.
> In any case, I do not think the DWIM would kick in when you try to detach
> at remote branch head. I did not check gitk code to find out the exact
> command line it uses, but I do not think it runs "checkout BRANCH". The
> command needs to be at least "checkout REMOTE/BRANCH" to work the way it
> does now with any released version of git, and I would not be surprised if
> paulus was cautious enough to have spelled it as "refs/REMOTE/BRANCH" to
> avoid any potential ambiguity issues.
Yes, I was confused when I wrote the original. I agree that "checkout
REMOTE/BRANCH" from the command line should still detach. If gitk wants
to prevent people accidentally detaching HEAD, the context menu for
remote branch boxes should probably detect remote branches and say
something like "Create local branch 'foo' from 'origin/foo'".
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
From: Johannes Sixt @ 2009-11-05 7:41 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1D8B9.9040603@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Johannes Sixt wrote:
>> It may be that I understand something incorrectly; but then I blame the
>> justification that you gave. In this case, it would be helpful to reword
>> the commit message, and perhaps add some results from your experiments.
>>
>
> The discussion which lead to this patch, including the experiments, can be
> found in the email thread starting here:
>
> http://thread.gmane.org/gmane.comp.version-control.git/129403
>
> (along with some other unrelated stuff; but it's not a long read :)
>
> In the above thread, Marius suggested API_WIN32, but I switched it around, since
> I thought it sounded better! I also thought about GIT_WIN32. Suggestions?
I suggested to treat WIN32 and _WIN32 as synonyms. The commit message
should summarize what you observed in your experiments.
But you can also tell me now why this is not possible. (I recall that your
report about the experiments was rather long; I don't have the time to
read and understand it again and to draw the correct conclusions.)
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Johannes Sixt @ 2009-11-05 7:35 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AF1D3A7.2070407@ramsay1.demon.co.uk>
Ramsay Jones schrieb:
> Johannes Sixt wrote:
>> I understand that you run into this error if you define NO_MMAP in your
>> config.mak. I don't know whether we support NO_MMAP as a knob for to tweak
>> the builds on all platforms. If this is the case (Junio?), then your
>> justification must be updated.
>
> AFAICT, the only build to not support NO_MMAP is MSVC (on cygwin *or* msysGit).
> The solution was obvious and low impact, so why not remove this anomaly?
Sure, why not? But then the justification is different, as I said.
-- Hannes
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Johannes Sixt @ 2009-11-05 7:33 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: git, Nicolas Pitre
In-Reply-To: <4AF214D5.6050202@gmail.com>
Andrzej K. Haczewski schrieb:
> This patch implements native to Windows subset of pthreads API used by Git.
> It allows to remove Pthreads for Win32 dependency for msysgit and cygwin.
>
> The patch modifies Makefile only for MSVC (that's the environment I'm
> capable of testing on), so it requires further corrections to compile
> with MinGW or Cygwin.
Looks quite good already.
In the next round, please squash this in.
diff --git a/Makefile b/Makefile
index bae1b40..6648d11 100644
--- a/Makefile
+++ b/Makefile
@@ -414,6 +414,7 @@ LIB_H += cache-tree.h
LIB_H += commit.h
LIB_H += compat/cygwin.h
LIB_H += compat/mingw.h
+LIB_H += compat/win32/pthread.h
LIB_H += csum-file.h
LIB_H += decorate.h
LIB_H += delta.h
^ permalink raw reply related
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-05 6:53 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911050016360.14365@iabervon.org>
On Thu, 5 Nov 2009, Daniel Barkalow wrote:
> On Thu, 5 Nov 2009, Sverre Rabbelier wrote:
>
> > Heya,
> >
> > On Wed, Nov 4, 2009 at 22:30, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> > >> On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > >> > That's not true for "git pull <url> <branch>"; we do want the remote ref,
> > >> > but it doesn't have a local peer.
> >
> > No, I don't think that's right, when doing a fetch we want to store
> > the refs somewhere, sure, but not under 'refs/heads/<branch>', perhaps
> > 'refs/hg/fetch/<branch>', either way, the current code does not work.
>
> I think you've still got things backwards. From the point of view of
> transport.c, refs/<vcs> is entirely opaque, and we never look at it. Those
> aren't local peers. They're a way for the helper to communicate to
> transport-helper.c. The user says: pull refs/heads/master of this hg
> repo. Transport.c tries to fetch refs/heads/master and get the sha1 to
> write into FETCH_HEAD or wherever. Transport-helper.c says "import
> refs/heads/master", and git-fast-import reads the resulting script and
> writes some ref that the helper tells it to write. Then transport-helper.c
> figures out where the ref was written, reads it, and updates the struct
> ref representing the remote info. Then builtin-fetch looks at the struct
> ref and writes it to the local repositories ref namespace or FETCH_HEAD.
>
> > >> >I think going straight to the refspec
> > >> > command is the right answer.
> > >>
> > >> Can you clarity what you mean with "the refspec command"?
> > >
> > > Whatever it is that lets the helper tell the transport code where in the
> > > helper's private namespace to look for refs. I'd been thinking the helper
> > > would advertize the "refspec" capability, and the transport code would
> > > call the "refspec" command in order to get the helper to report that; but
> > > then I actually only said that the helper reports refspec, and not
> > > proposed a name for the command.
> >
> > Currently I'm implementing so that it would work like this for the svn helper:
> >
> > $ echo capabilities | git remote-svn origin /path/to/hg/repo
> > import
> > refspec +refs/trunk:refs/svn/origin/trunk
> > refspec +refs/branches/*:refs/svn/origin/*
> >
> > That way we can put the refspec in the config file at clone time.
> >
> > Now I've been browsing through the builtin-fetch code, and it looks
> > like the main problem is going to be to apply this refspec at all.
> > I'll have a more extensive look tomorrow.
>
> This is entirely not what I think we should have. The config file should
> say refs/heads/*:refs/remotes/origin/* like it always does, because the
> transport will list the refs like refs/heads/* and refs/tags/* and return
> them like that.
>
> I'll see if I can write up an untested patch that does what I'm thinking
> of.
Here's a patch (on my original series, which doesn't seem to be in pu any
more, but should be floating around somewhere). Completely untested,
except that it compiles. The idea is that the helper will say something
like:
refspec refs/heads/master:refs/svn/origin/trunk
refspec refs/heads/*:refs/svn/origin/branches/*
refspec refs/tags/*:refs/svn/origin/tags/*
and transport-helper will use these patterns to figure out where to find
the correct value from the helper's private namespace when asked to fetch
refs/heads/topic.
-Daniel
commit 483836f6411d2317f24c7594c557fb01133508b6
Author: Daniel Barkalow <barkalow@iabervon.org>
Date: Thu Nov 5 01:39:02 2009 -0500
Allow helper to map private ref names into normal names
This allows a helper to say that, when it handles "import
refs/heads/topic", the script it outputs will actually write to
refs/svn/origin/branches/topic; therefore, transport-helper should
read it from the latter location after git-fast-import completes.
diff --git a/remote.c b/remote.c
index f0441c4..58d1a61 100644
--- a/remote.c
+++ b/remote.c
@@ -790,6 +790,24 @@ static int match_name_with_pattern(const char *key, const char *name,
return ret;
}
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+ const char *name)
+{
+ int i;
+ char *ret = NULL;
+ for (i = 0; i < nr_refspec; i++) {
+ struct refspec *refspec = refspecs + i;
+ if (refspec->pattern) {
+ if (match_name_with_pattern(refspec->src, name,
+ refspec->dst, &ret))
+ return ret;
+ } else if (!strcmp(refspec->src, name))
+ return strdup(refspec->dst);
+ }
+ return NULL;
+
+}
+
int remote_find_tracking(struct remote *remote, struct refspec *refspec)
{
int find_src = refspec->src == NULL;
diff --git a/remote.h b/remote.h
index ac0ce2f..c2f920b 100644
--- a/remote.h
+++ b/remote.h
@@ -91,6 +91,9 @@ void ref_remove_duplicates(struct ref *ref_map);
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+ const char *name);
+
int match_refs(struct ref *src, struct ref **dst,
int nr_refspec, const char **refspec, int all);
diff --git a/transport-helper.c b/transport-helper.c
index aa5ad3c..88573e7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -5,6 +5,7 @@
#include "commit.h"
#include "diff.h"
#include "revision.h"
+#include "remote.h"
struct helper_data
{
@@ -12,6 +13,9 @@ struct helper_data
struct child_process *helper;
unsigned fetch : 1;
unsigned import : 1;
+ /* These go from remote name (as in "list") to private name */
+ struct refspec *refspecs;
+ int refspec_nr;
};
static struct child_process *get_helper(struct transport *transport)
@@ -20,6 +24,9 @@ static struct child_process *get_helper(struct transport *transport)
struct strbuf buf = STRBUF_INIT;
struct child_process *helper;
FILE *file;
+ const char **refspecs = NULL;
+ int refspec_nr = 0;
+ int refspec_alloc = 0;
if (data->helper)
return data->helper;
@@ -53,6 +60,16 @@ static struct child_process *get_helper(struct transport *transport)
data->fetch = 1;
if (!strcmp(buf.buf, "import"))
data->import = 1;
+ if (!prefixcmp(buf.buf, "refspec ")) {
+ ALLOC_GROW(refspecs,
+ refspec_nr + 1,
+ refspec_alloc);
+ refspecs[refspec_nr++] = strdup(buf.buf + strlen("refspec "));
+ }
+ }
+ if (refspecs) {
+ data->refspec_nr = refspec_nr;
+ data->refspecs = parse_fetch_refspec(refspec_nr, refspecs);
}
return data->helper;
}
@@ -121,6 +138,7 @@ static int fetch_with_import(struct transport *transport,
{
struct child_process fastimport;
struct child_process *helper = get_helper(transport);
+ struct helper_data *data = transport->data;
int i;
struct ref *posn;
struct strbuf buf = STRBUF_INIT;
@@ -141,10 +159,16 @@ static int fetch_with_import(struct transport *transport,
finish_command(&fastimport);
for (i = 0; i < nr_heads; i++) {
+ char *private;
posn = to_fetch[i];
if (posn->status & REF_STATUS_UPTODATE)
continue;
- read_ref(posn->name, posn->old_sha1);
+ if (data->refspecs)
+ private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
+ else
+ private = strdup(posn->name);
+ read_ref(private, posn->old_sha1);
+ free(private);
}
return 0;
}
^ permalink raw reply related
* [PATCHv2 2/2] t1200: Make documentation and test agree
From: Stephen Boyd @ 2009-11-05 6:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1257402833-4741-1-git-send-email-bebarino@gmail.com>
There were some differences between t1200 and the gitcore-tutorial. Add
missing tests for manually merging two branches, and use the same
commands in both files.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
This is all new since v1
Documentation/gitcore-tutorial.txt | 20 ++++----
t/t1200-tutorial.sh | 97 ++++++++++++++++++++++++++++++++---
2 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index b3640c4..7bdf090 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -185,7 +185,7 @@ object is. git will tell you that you have a "blob" object (i.e., just a
regular file), and you can see the contents with
----------------
-$ git cat-file "blob" 557db03
+$ git cat-file blob 557db03
----------------
which will print out "Hello World". The object `557db03` is nothing
@@ -1188,7 +1188,7 @@ $ git show-branch
--
+ [mybranch] Some work.
* [master] Some fun.
-*+ [mybranch^] New day.
+*+ [mybranch^] Initial commit
------------
Now we are ready to experiment with the merge by hand.
@@ -1204,11 +1204,11 @@ $ mb=$(git merge-base HEAD mybranch)
The command writes the commit object name of the common ancestor
to the standard output, so we captured its output to a variable,
because we will be using it in the next step. By the way, the common
-ancestor commit is the "New day." commit in this case. You can
+ancestor commit is the "Initial commit" commit in this case. You can
tell it by:
------------
-$ git name-rev $mb
+$ git name-rev --name-only --tags $mb
my-first-tag
------------
@@ -1237,8 +1237,8 @@ inspect the index file with this command:
------------
$ git ls-files --stage
100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
@@ -1253,8 +1253,8 @@ To look at only non-zero stages, use `\--unmerged` flag:
------------
$ git ls-files --unmerged
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
@@ -1283,8 +1283,8 @@ the working tree.. This can be seen if you run `ls-files
------------
$ git ls-files --stage
100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
-100644 263414f423d0e4d70dae8fe53fa34614ff3e2860 1 hello
-100644 06fa6a24256dc7e560efa5687fa84b51f0263c3a 2 hello
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
------------
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index c57c9d5..299e724 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -47,8 +47,9 @@ test_expect_success 'tree' '
'
test_expect_success 'git diff-index -p HEAD' '
- echo "Initial commit" | \
- git commit-tree $(git write-tree) 2>&1 > .git/refs/heads/master &&
+ tree=$(git write-tree)
+ commit=$(echo "Initial commit" | git commit-tree $tree) &&
+ git update-ref HEAD $commit &&
git diff-index -p HEAD > diff.output &&
cmp diff.expect diff.output
'
@@ -131,16 +132,18 @@ Work, work, work
EOF
cat > show-branch.expect << EOF
-* [master] Merged "mybranch" changes.
+* [master] Merge work in mybranch
! [mybranch] Some work.
--
-- [master] Merged "mybranch" changes.
+- [master] Merge work in mybranch
*+ [mybranch] Some work.
+* [master^] Some fun.
EOF
test_expect_success 'git show-branch' '
- git commit -m "Merged \"mybranch\" changes." -i hello &&
- git show-branch --topo-order master mybranch > show-branch.output &&
+ git commit -m "Merge work in mybranch" -i hello &&
+ git show-branch --topo-order --more=1 master mybranch \
+ > show-branch.output &&
cmp show-branch.expect show-branch.output
'
@@ -160,10 +163,10 @@ test_expect_success 'git resolve' '
'
cat > show-branch2.expect << EOF
-! [master] Merged "mybranch" changes.
- * [mybranch] Merged "mybranch" changes.
+! [master] Merge work in mybranch
+ * [mybranch] Merge work in mybranch
--
--- [master] Merged "mybranch" changes.
+-- [master] Merge work in mybranch
EOF
test_expect_success 'git show-branch (part 2)' '
@@ -171,6 +174,82 @@ test_expect_success 'git show-branch (part 2)' '
cmp show-branch2.expect show-branch2.output
'
+cat > show-branch3.expect << EOF
+! [master] Merge work in mybranch
+ * [mybranch] Merge work in mybranch
+--
+-- [master] Merge work in mybranch
++* [master^2] Some work.
++* [master^] Some fun.
+EOF
+
+test_expect_success 'git show-branch (part 3)' '
+ git show-branch --topo-order --more=2 master mybranch \
+ > show-branch3.output &&
+ cmp show-branch3.expect show-branch3.output
+'
+
+test_expect_success 'rewind to "Some fun." and "Some work."' '
+ git checkout mybranch &&
+ git reset --hard master^2 &&
+ git checkout master &&
+ git reset --hard master^
+'
+
+cat > show-branch4.expect << EOF
+* [master] Some fun.
+ ! [mybranch] Some work.
+--
+ + [mybranch] Some work.
+* [master] Some fun.
+*+ [mybranch^] Initial commit
+EOF
+
+test_expect_success 'git show-branch (part 4)' '
+ git show-branch --topo-order > show-branch4.output &&
+ cmp show-branch4.expect show-branch4.output
+'
+
+test_expect_success 'manual merge' '
+ mb=$(git merge-base HEAD mybranch) &&
+ git name-rev --name-only --tags $mb > name-rev.output &&
+ test "my-first-tag" = $(cat name-rev.output) &&
+
+ git read-tree -m -u $mb HEAD mybranch
+'
+
+cat > ls-files.expect << EOF
+100644 7f8b141b65fdcee47321e399a2598a235a032422 0 example
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
+100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
+EOF
+
+test_expect_success 'git ls-files --stage' '
+ git ls-files --stage > ls-files.output &&
+ cmp ls-files.expect ls-files.output
+'
+
+cat > ls-files-unmerged.expect << EOF
+100644 557db03de997c86a4a028e1ebd3a1ceb225be238 1 hello
+100644 ba42a2a96e3027f3333e13ede4ccf4498c3ae942 2 hello
+100644 cc44c73eb783565da5831b4d820c962954019b69 3 hello
+EOF
+
+test_expect_success 'git ls-files --unmerged' '
+ git ls-files --unmerged > ls-files-unmerged.output &&
+ cmp ls-files-unmerged.expect ls-files-unmerged.output
+'
+
+test_expect_success 'git-merge-index' '
+ test_must_fail git merge-index git-merge-one-file hello
+'
+
+test_expect_success 'git ls-files --stage (part 2)' '
+ git ls-files --stage > ls-files.output2 &&
+ cmp ls-files.expect ls-files.output2
+'
+
test_expect_success 'git repack' 'git repack'
test_expect_success 'git prune-packed' 'git prune-packed'
test_expect_success '-> only packed objects' '
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* [PATCHv2 1/2] t1200: cleanup and modernize test style
From: Stephen Boyd @ 2009-11-05 6:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1257282328-6491-1-git-send-email-bebarino@gmail.com>
Many parts of the tests in t1200 are run outside the test harness,
circumventing the usefulness of -v and spewing messages to stdout when
-v isn't used. Fix these problems by modernizing the test a bit.
An extra test_done has existed since commit 6a74642 (git-commit --amend:
two fixes., 2006-04-20) leading to the last 6 tests never being run.
Remove it and teach the resolve merge test about fast-forward merges.
Also fix the last test's incorrect find command and prune before
checking for unpacked objects so we remove the unreachable conflict-marked
blob.
Finally, we remove the TODO notes, because fetch, push, and clone have
their own tests since t1200 was introduced and we're not going to add
them here 4 years later.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
Since v1:
* Fixed the last test
t/t1200-tutorial.sh | 134 +++++++++++++++++++++++++++++----------------------
1 files changed, 76 insertions(+), 58 deletions(-)
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index 67e637b..c57c9d5 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -7,14 +7,18 @@ test_description='A simple turial in the form of a test case'
. ./test-lib.sh
-echo "Hello World" > hello
-echo "Silly example" > example
+test_expect_success 'blob' '
+ echo "Hello World" > hello &&
+ echo "Silly example" > example &&
-git update-index --add hello example
+ git update-index --add hello example &&
-test_expect_success 'blob' "test blob = \"$(git cat-file -t 557db03)\""
+ test blob = "$(git cat-file -t 557db03)"
+'
-test_expect_success 'blob 557db03' "test \"Hello World\" = \"$(git cat-file blob 557db03)\""
+test_expect_success 'blob 557db03' '
+ test "Hello World" = "$(git cat-file blob 557db03)"
+'
echo "It's a new day for git" >>hello
cat > diff.expect << EOF
@@ -26,25 +30,33 @@ index 557db03..263414f 100644
Hello World
+It's a new day for git
EOF
-git diff-files -p > diff.output
-test_expect_success 'git diff-files -p' 'cmp diff.expect diff.output'
-git diff > diff.output
-test_expect_success 'git diff' 'cmp diff.expect diff.output'
-
-tree=$(git write-tree 2>/dev/null)
-test_expect_success 'tree' "test 8988da15d077d4829fc51d8544c097def6644dbb = $tree"
+test_expect_success 'git diff-files -p' '
+ git diff-files -p > diff.output &&
+ cmp diff.expect diff.output
+'
-output="$(echo "Initial commit" | git commit-tree $(git write-tree) 2>&1 > .git/refs/heads/master)"
+test_expect_success 'git diff' '
+ git diff > diff.output &&
+ cmp diff.expect diff.output
+'
-git diff-index -p HEAD > diff.output
-test_expect_success 'git diff-index -p HEAD' 'cmp diff.expect diff.output'
+test_expect_success 'tree' '
+ tree=$(git write-tree 2>/dev/null)
+ test 8988da15d077d4829fc51d8544c097def6644dbb = $tree
+'
-git diff HEAD > diff.output
-test_expect_success 'git diff HEAD' 'cmp diff.expect diff.output'
+test_expect_success 'git diff-index -p HEAD' '
+ echo "Initial commit" | \
+ git commit-tree $(git write-tree) 2>&1 > .git/refs/heads/master &&
+ git diff-index -p HEAD > diff.output &&
+ cmp diff.expect diff.output
+'
-#rm hello
-#test_expect_success 'git read-tree --reset HEAD' "git read-tree --reset HEAD ; test \"hello: needs update\" = \"$(git update-index --refresh)\""
+test_expect_success 'git diff HEAD' '
+ git diff HEAD > diff.output &&
+ cmp diff.expect diff.output
+'
cat > whatchanged.expect << EOF
commit VARIABLE
@@ -69,39 +81,45 @@ index 0000000..557db03
+Hello World
EOF
-git whatchanged -p --root | \
- sed -e "1s/^\(.\{7\}\).\{40\}/\1VARIABLE/" \
+test_expect_success 'git whatchanged -p --root' '
+ git whatchanged -p --root | \
+ sed -e "1s/^\(.\{7\}\).\{40\}/\1VARIABLE/" \
-e "2,3s/^\(.\{8\}\).*$/\1VARIABLE/" \
-> whatchanged.output
-test_expect_success 'git whatchanged -p --root' 'cmp whatchanged.expect whatchanged.output'
-
-git tag my-first-tag
-test_expect_success 'git tag my-first-tag' 'cmp .git/refs/heads/master .git/refs/tags/my-first-tag'
+ > whatchanged.output &&
+ cmp whatchanged.expect whatchanged.output
+'
-# TODO: test git clone
+test_expect_success 'git tag my-first-tag' '
+ git tag my-first-tag &&
+ cmp .git/refs/heads/master .git/refs/tags/my-first-tag
+'
-git checkout -b mybranch
-test_expect_success 'git checkout -b mybranch' 'cmp .git/refs/heads/master .git/refs/heads/mybranch'
+test_expect_success 'git checkout -b mybranch' '
+ git checkout -b mybranch &&
+ cmp .git/refs/heads/master .git/refs/heads/mybranch
+'
cat > branch.expect <<EOF
master
* mybranch
EOF
-git branch > branch.output
-test_expect_success 'git branch' 'cmp branch.expect branch.output'
+test_expect_success 'git branch' '
+ git branch > branch.output &&
+ cmp branch.expect branch.output
+'
-git checkout mybranch
-echo "Work, work, work" >>hello
-git commit -m 'Some work.' -i hello
+test_expect_success 'git resolve now fails' '
+ git checkout mybranch &&
+ echo "Work, work, work" >>hello &&
+ git commit -m "Some work." -i hello &&
-git checkout master
+ git checkout master &&
-echo "Play, play, play" >>hello
-echo "Lots of fun" >>example
-git commit -m 'Some fun.' -i hello example
+ echo "Play, play, play" >>hello &&
+ echo "Lots of fun" >>example &&
+ git commit -m "Some fun." -i hello example &&
-test_expect_success 'git resolve now fails' '
test_must_fail git merge -m "Merge work in mybranch" mybranch
'
@@ -112,10 +130,6 @@ Play, play, play
Work, work, work
EOF
-git commit -m 'Merged "mybranch" changes.' -i hello
-
-test_done
-
cat > show-branch.expect << EOF
* [master] Merged "mybranch" changes.
! [mybranch] Some work.
@@ -124,21 +138,26 @@ cat > show-branch.expect << EOF
*+ [mybranch] Some work.
EOF
-git show-branch --topo-order master mybranch > show-branch.output
-test_expect_success 'git show-branch' 'cmp show-branch.expect show-branch.output'
-
-git checkout mybranch
+test_expect_success 'git show-branch' '
+ git commit -m "Merged \"mybranch\" changes." -i hello &&
+ git show-branch --topo-order master mybranch > show-branch.output &&
+ cmp show-branch.expect show-branch.output
+'
cat > resolve.expect << EOF
-Updating from VARIABLE to VARIABLE
+Updating VARIABLE..VARIABLE
+Fast forward (no commit created; -m option ignored)
example | 1 +
hello | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
EOF
-git merge -s "Merge upstream changes." master | \
- sed -e "1s/[0-9a-f]\{40\}/VARIABLE/g" >resolve.output
-test_expect_success 'git resolve' 'cmp resolve.expect resolve.output'
+test_expect_success 'git resolve' '
+ git checkout mybranch &&
+ git merge -m "Merge upstream changes." master | \
+ sed -e "1s/[0-9a-f]\{7\}/VARIABLE/g" >resolve.output &&
+ cmp resolve.expect resolve.output
+'
cat > show-branch2.expect << EOF
! [master] Merged "mybranch" changes.
@@ -147,17 +166,16 @@ cat > show-branch2.expect << EOF
-- [master] Merged "mybranch" changes.
EOF
-git show-branch --topo-order master mybranch > show-branch2.output
-test_expect_success 'git show-branch' 'cmp show-branch2.expect show-branch2.output'
-
-# TODO: test git fetch
-
-# TODO: test git push
+test_expect_success 'git show-branch (part 2)' '
+ git show-branch --topo-order master mybranch > show-branch2.output &&
+ cmp show-branch2.expect show-branch2.output
+'
test_expect_success 'git repack' 'git repack'
test_expect_success 'git prune-packed' 'git prune-packed'
test_expect_success '-> only packed objects' '
- ! find -type f .git/objects/[0-9a-f][0-9a-f]
+ git prune && # Remove conflict marked blobs
+ ! find .git/objects/[0-9a-f][0-9a-f] -type f
'
test_done
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* What's cooking in git.git (Nov 2009, #01; Wed, 04)
From: Junio C Hamano @ 2009-11-05 5:41 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'. The ones
marked with '.' do not appear in any of the integration branches, but I am
still holding onto them.
In 1.7.0, we plan to correct handful of warts in the interfaces everybody
agrees that they were mistakes. The resulting system may not be strictly
backward compatible. Currently planeed changes are:
* refuse push to update the checked out branch in a non-bare repo by
default
Make "git push" into a repository to update the branch that is checked
out fail by default. You can countermand this default by setting a
configuration variable in the receiving repository.
http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007
* refuse push to delete the current branch by default
Make "git push $there :$killed" to delete the branch that is pointed at
by its HEAD fail by default. You can countermand this default by
setting a configuration variable in the receiving repository.
http://thread.gmane.org/gmane.comp.version-control.git/108862/focus=108936
* git-send-email won't make deep threads by default
Many people said that by default when sending more than 2 patches the
threading git-send-email makes by default is hard to read, and they
prefer the default be one cover letter and each patch as a direct
follow-up to the cover letter. You can countermand this by setting a
configuration variable.
http://article.gmane.org/gmane.comp.version-control.git/109790
* git-status won't be "git-commit --dry-run" anymore
http://thread.gmane.org/gmane.comp.version-control.git/125989/focus=125993
* "git-diff -w --exit-code" will exit success if only differences it
found are whitespace changes that are stripped away from the output.
http://thread.gmane.org/gmane.comp.version-control.git/119731/focus=119751
--------------------------------------------------
[New Topics]
* bw/autoconf-more (2009-11-04) 2 commits
- configure: add settings for gitconfig, editor and pager
- configure: add macro to set arbitrary make variables
* em/commit-claim (2009-11-04) 1 commit
- commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
I just picked better bits from both versions.
* jk/maint-format-patch-p-suppress-stat (2009-11-04) 2 commits.
- format-patch: make "-p" suppress diffstat
- Revert "format-patch -p is now a no-op" series
(this branch uses bg/format-patch-p-noop.)
This corrects a mistake made soon after 1.6.0.
* rj/maint-simplify-cygwin-makefile (2009-10-27) 1 commit.
- Makefile: merge two Cygwin configuration sections into one
This is one of the most obviously correct bit from "Compiling on Cygwin
using MSVC fails" topic I didn't really look at. If J6t is Ok with the
series, I don't mind queueing the whole thing myself.
* vl/maint-openssl-signature-change (2009-10-31) 1 commit.
(merged to 'next' on 2009-10-31 at 0e1ce6b)
+ imap-send.c: fix compiler warnings for OpenSSL 1.0
Prepare ourselves before newer versions of OpenSSL hits more platforms.
--------------------------------------------------
[Stalled]
* tr/filter-branch (2009-10-28) 2 commits.
- filter-branch: nearest-ancestor rewriting outside subdir filter
- filter-branch: stop special-casing $filter_subdir argument
J6t had some comments on this.
* ne/rev-cache (2009-10-19) 7 commits.
- support for commit grafts, slight change to general mechanism
- support for path name caching in rev-cache
- full integration of rev-cache into git, completed test suite
- administrative functions for rev-cache, start of integration into git
- support for non-commit object caching in rev-cache
- basic revision cache system, no integration or features
- man page and technical discussion for rev-cache
The author indicated that there is another round coming. Does not seem to
pass the tests when merged to 'pu'.
* jl/submodule-add-noname (2009-09-22) 1 commit.
- git submodule add: make the <path> parameter optional
Dscho started an interesting discussion regarding the larger workflow in
which the "submodule add" is used. I think the patch itself makes sense
but at the same time it probably makes sense to also take the <path> and
infer the <repository> as Dscho suggested, probably in "git submodule
add", not in "git add" proper, at least initially.
* sr/gfi-options (2009-09-06) 6 commits.
- fast-import: test the new option command
- fast-import: add option command
- fast-import: test the new feature command
- fast-import: add feature command
- fast-import: put marks reading in it's own function
- fast-import: put option parsing code in separate functions
Seems to be moving again soon.
* je/send-email-no-subject (2009-08-05) 1 commit.
(merged to 'next' on 2009-10-11 at 1b99c56)
+ send-email: confirm on empty mail subjects
The existing tests cover the positive case (i.e. as long as the user says
"yes" to the "do you really want to send this message that lacks subject",
the message is sent) of this feature, but the feature itself needs its own
test to verify the negative case (i.e. does it correctly stop if the user
says "no"?)
--------------------------------------------------
[Cooking]
* bg/merge-ff-only (2009-10-29) 1 commit
(merged to 'next' on 2009-10-31 at b6b49aa)
+ Teach 'git merge' and 'git pull' the option --ff-only
* jk/maint-1.6.3-ls-files-i (2009-10-30) 1 commit.
(merged to 'next' on 2009-10-31 at 3a31fcc)
+ ls-files: unbreak "ls-files -i"
* jn/editor-pager (2009-10-30) 8 commits
- Provide a build time default-pager setting
- Provide a build time default-editor setting
- am -i, git-svn: use "git var GIT_PAGER"
- add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
- Teach git var about GIT_PAGER
- Teach git var about GIT_EDITOR
- Do not use VISUAL editor on dumb terminals
- Handle more shell metacharacters in editor names
* js/maint-diff-color-words (2009-10-30) 3 commits.
- diff --color-words: bit of clean-up
- diff --color-words -U0: fix the location of hunk headers
- t4034-diff-words: add a test for word diff without context
Fixes a corner case of running --color-words with -U0.
* sc/difftool-p4merge (2009-10-28) 1 commit
(merged to 'next' on 2009-10-31 at 194b5c5)
+ mergetool--lib: add p4merge as a pre-configured mergetool option
* sc/protocol-doc (2009-10-29) 1 commit
- Update packfile transfer protocol documentation
There is the final draft posted, but I haven't picked it up yet.
* sr/vcs-helper (2009-11-04) 13 commits
- Add Python support library for remote helpers
- Basic build infrastructure for Python scripts
- Allow helpers to request the path to the .git directory
- Allow helpers to report in "list" command that the ref is unchanged
- Honour the refspec when updating refs after import
- Write local refs written by the "import" helper command only once
- Add support for "import" helper command
- Allow specifying the remote helper in the url
- Add a config option for remotes to specify a foreign vcs
- Allow fetch to modify refs
- Use a function to determine whether a remote is valid
- Allow programs to not depend on remotes having urls
- Fix memory leak in helper method for disconnect
Supposed to replace db/vcs-helper-rest. Still does not pass tests in
'pu'.
* tr/describe-advice (2009-10-28) 1 commit
(merged to 'next' on 2009-10-31 at 8084850)
+ describe: when failing, tell the user about options that work
* mr/gitweb-snapshot (2009-10-29) 3 commits.
- gitweb: Smarter snapshot names
- t/gitweb-lib.sh: Split gitweb output into headers and body
(merged to 'next' on 2009-10-11 at 22ba047)
+ gitweb: check given hash before trying to create snapshot
Replaced the tip with Jakub's updates.
* jp/dirty-describe (2009-10-21) 1 commit.
(merged to 'next' on 2009-10-30 at 19c7fc7)
+ Teach "git describe" --dirty option
* jp/fetch-cull-many-refs (2009-10-25) 2 commits
(merged to 'next' on 2009-11-01 at 1f09ce9)
+ fetch: Speed up fetch of large numbers of refs
+ remote: Make ref_remove_duplicates faster for large numbers of refs
* bg/format-patch-p-noop (2009-10-25) 3 commits.
(merged to 'next' on 2009-10-30 at e34a3db)
+ format-patch documentation: Fix formatting
+ format-patch documentation: Remove diff options that are not useful
+ format-patch: Make implementation and documentation agree
(this branch is used by jk/maint-format-patch-p-suppress-stat.)
Will revert from 'next' by merging Peff's fix.
* jk/gitignore-anchored (2009-10-26) 1 commit
(merged to 'next' on 2009-10-30 at 9391a93)
+ gitignore: root most patterns at the top-level directory
* jk/maint-add-p-empty (2009-10-27) 1 commit.
(merged to 'next' on 2009-10-30 at 2bd302f)
+ add-interactive: handle deletion of empty files
* jk/maint-push-config (2009-10-25) 1 commit.
(merged to 'next' on 2009-10-30 at 934e3c5)
+ push: always load default config
* lt/revision-bisect (2009-10-27) 1 commit.
(merged to 'next' on 2009-10-30 at 81ee52b)
+ Add '--bisect' revision machinery argument
* jc/pretty-lf (2009-10-04) 1 commit.
- Pretty-format: %[+-]x to tweak inter-item newlines
* rs/pretty-wrap (2009-10-17) 1 commit
(merged to 'next' on 2009-10-30 at 403bbfe)
+ Implement wrap format %w() as if it is a mode switch
(this branch uses js/log-rewrap.)
* js/log-rewrap (2009-10-18) 3 commits
(merged to 'next' on 2009-10-30 at 403bbfe)
+ Teach --wrap to only indent without wrapping
+ Add strbuf_add_wrapped_text() to utf8.[ch]
+ print_wrapped_text(): allow hard newlines
(this branch is used by rs/pretty-wrap.)
* sr/blame-incomplete (2009-10-19) 1 commit.
(merged to 'next' on 2009-10-22 at 133e0ce)
+ blame: make sure that the last line ends in an LF
I think this is _good enough_ as-is; although it would be better if we
added some hint to the output for Porcelain implementations, that can be
done as a follow-up fix.
* fc/doc-fast-forward (2009-10-24) 1 commit.
(merged to 'next' on 2009-11-01 at faaad90)
+ Use 'fast-forward' all over the place
* ks/precompute-completion (2009-10-26) 3 commits.
(merged to 'next' on 2009-10-28 at cd5177f)
+ completion: ignore custom merge strategies when pre-generating
(merged to 'next' on 2009-10-22 at f46a28a)
+ bug: precomputed completion includes scripts sources
(merged to 'next' on 2009-10-14 at adf722a)
+ Speedup bash completion loading
* sp/smart-http (2009-11-04) 30 commits
- http-backend: Test configuration options
- http-backend: Use http.getanyfile to disable dumb HTTP serving
- test smart http fetch and push
- http tests: use /dumb/ URL prefix
- set httpd port before sourcing lib-httpd
- t5540-http-push: remove redundant fetches
- Smart HTTP fetch: gzip requests
- Smart fetch over HTTP: client side
- Smart push over HTTP: client side
- Discover refs via smart HTTP server when available
- http-backend: more explict LocationMatch
- http-backend: add example for gitweb on same URL
- http-backend: use mod_alias instead of mod_rewrite
- http-backend: reword some documentation
- http-backend: add GIT_PROJECT_ROOT environment var
- Smart fetch and push over HTTP: server side
- Add stateless RPC options to upload-pack, receive-pack
- Git-aware CGI to provide dumb HTTP transport
- remote-helpers: return successfully if everything up-to-date
- Move WebDAV HTTP push under remote-curl
- remote-helpers: Support custom transport options
- remote-helpers: Fetch more than one ref in a batch
- fetch: Allow transport -v -v -v to set verbosity to 3
- remote-curl: Refactor walker initialization
- Add multi_ack_detailed capability to fetch-pack/upload-pack
- Move "get_ack()" back to fetch-pack
- fetch-pack: Use a strbuf to compose the want list
- pkt-line: Make packet_read_line easier to debug
- pkt-line: Add strbuf based functions
- http-push: fix check condition on http.c::finish_http_pack_request()
v5 plus 3 more fix-up patches from today.
* ef/msys-imap (2009-10-22) 9 commits.
(merged to 'next' on 2009-10-31 at 8630603)
+ Windows: use BLK_SHA1 again
+ MSVC: Enable OpenSSL, and translate -lcrypto
+ mingw: enable OpenSSL
+ mingw: wrap SSL_set_(w|r)fd to call _get_osfhandle
+ imap-send: build imap-send on Windows
+ imap-send: fix compilation-error on Windows
+ imap-send: use run-command API for tunneling
+ imap-send: use separate read and write fds
+ imap-send: remove useless uid code
* jc/fix-tree-walk (2009-10-22) 11 commits.
(merged to 'next' on 2009-10-22 at 10c0c8f)
+ Revert failed attempt since 353c5ee
+ read-tree --debug-unpack
(merged to 'next' on 2009-10-11 at 0b058e2)
+ unpack-trees.c: look ahead in the index
+ unpack-trees.c: prepare for looking ahead in the index
+ Aggressive three-way merge: fix D/F case
+ traverse_trees(): handle D/F conflict case sanely
+ more D/F conflict tests
+ tests: move convenience regexp to match object names to test-lib.sh
+ unpack_callback(): use unpack_failed() consistently
+ unpack-trees: typofix
+ diff-lib.c: fix misleading comments on oneway_diff()
This has some stupid bugs and temporarily reverted from 'next' until I can
fix it, but the "temporarily" turned out to be very loooong. Sigh...
* jh/notes (2009-10-09) 22 commits.
- fast-import: Proper notes tree manipulation using the notes API
- Refactor notes concatenation into a flexible interface for combining notes
- Notes API: Allow multiple concurrent notes trees with new struct notes_tree
- Notes API: for_each_note(): Traverse the entire notes tree with a callback
- Notes API: get_note(): Return the note annotating the given object
- Notes API: add_note(): Add note objects to the internal notes tree structure
- Notes API: init_notes(): Initialize the notes tree from the given notes ref
- Notes API: get_commit_notes() -> format_note() + remove the commit restriction
(merged to 'next' on 2009-11-01 at 948327a)
+ Add selftests verifying concatenation of multiple notes for the same commit
+ Refactor notes code to concatenate multiple notes annotating the same object
+ Add selftests verifying that we can parse notes trees with various fanouts
+ Teach the notes lookup code to parse notes trees with various fanout schemes
+ Teach notes code to free its internal data structures on request
+ Add '%N'-format for pretty-printing commit notes
+ Add flags to get_commit_notes() to control the format of the note string
+ t3302-notes-index-expensive: Speed up create_repo()
+ fast-import: Add support for importing commit notes
+ Teach "-m <msg>" and "-F <file>" to "git notes edit"
+ Add an expensive test for git-notes
+ Speed up git notes lookup
+ Add a script to edit/inspect notes
+ Introduce commit notes
* jn/gitweb-blame (2009-09-01) 5 commits.
- gitweb: Minify gitweb.js if JSMIN is defined
- gitweb: Create links leading to 'blame_incremental' using JavaScript
(merged to 'next' on 2009-10-11 at 73c4a83)
+ gitweb: Colorize 'blame_incremental' view during processing
+ gitweb: Incremental blame (using JavaScript)
+ gitweb: Add optional "time to generate page" info in footer
Ajax-y blame.
* nd/sparse (2009-08-20) 19 commits.
- sparse checkout: inhibit empty worktree
- Add tests for sparse checkout
- read-tree: add --no-sparse-checkout to disable sparse checkout support
- unpack-trees(): ignore worktree check outside checkout area
- unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
- unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
- unpack-trees.c: generalize verify_* functions
- unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
- Introduce "sparse checkout"
- dir.c: export excluded_1() and add_excludes_from_file_1()
- excluded_1(): support exclude files in index
- unpack-trees(): carry skip-worktree bit over in merged_entry()
- Read .gitignore from index if it is skip-worktree
- Avoid writing to buffer in add_excludes_from_file_1()
- Teach Git to respect skip-worktree bit (writing part)
- Teach Git to respect skip-worktree bit (reading part)
- Introduce "skip-worktree" bit in index, teach Git to get/set this bit
- Add test-index-version
- update-index: refactor mark_valid() in preparation for new options
--------------------------------------------------
[For 1.7.0]
* jc/1.7.0-no-commit-no-ff-2 (2009-10-22) 1 commit.
- git-merge: forbid fast-forward and up-to-date when --no-commit is given
This makes "git merge --no-commit" fail when it results in fast-forward or
up-to-date. I haven't described this at the beginning of this message
yet, as it is not clear if this change is even necessary. Opinions?
* jk/1.7.0-status (2009-09-05) 5 commits.
- docs: note that status configuration affects only long format
(merged to 'next' on 2009-10-11 at 65c8513)
+ commit: support alternate status formats
+ status: add --porcelain output format
+ status: refactor format option parsing
+ status: refactor short-mode printing to its own function
(this branch uses jc/1.7.0-status.)
Gives the --short output format to post 1.7.0 "git commit --dry-run" that
is similar to that of post 1.7.0 "git status".
The tip one is not in 'next' as I have been hoping that somebody may want
to change the code to make it unnecessary, but it does not seem to be
happening, so probably it should also go to 'next'.
* jc/1.7.0-status (2009-09-05) 4 commits.
(merged to 'next' on 2009-10-11 at 9558627)
+ status: typo fix in usage
+ git status: not "commit --dry-run" anymore
+ git stat -s: short status output
+ git stat: the beginning of "status that is not a dry-run of commit"
(this branch is used by jk/1.7.0-status.)
With this, "git status" is no longer "git commit --dry-run".
* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit.
(merged to 'next' on 2009-10-11 at 043acdf)
+ send-email: make --no-chain-reply-to the default
* jc/1.7.0-diff-whitespace-only-status (2009-08-30) 4 commits.
(merged to 'next' on 2009-10-11 at 546c74d)
+ diff.c: fix typoes in comments
+ Make test case number unique
+ diff: Rename QUIET internal option to QUICK
+ diff: change semantics of "ignore whitespace" options
This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output. It is a backward incompatible change, but
we could argue that it is a bugfix.
* jc/1.7.0-push-safety (2009-02-09) 2 commits.
(merged to 'next' on 2009-10-11 at 81b8128)
+ Refuse deleting the current branch via push
+ Refuse updating the current branch in a non-bare repository via push
--------------------------------------------------
[I have been too busy to purge these]
* jc/log-tz (2009-03-03) 1 commit.
- Allow --date=local --date=other-format to work as expected
Maybe some people care about this. I dunno.
* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
- mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker
Maybe some people care about this. I dunno.
* jg/log-format-body-indent (2009-09-19) 1 commit.
. git-log --format: Add %B tag with %B(x) option
^ permalink raw reply
* Re: [PATCH v2] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Junio C Hamano @ 2009-11-05 5:40 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Erick Mattos, git
In-Reply-To: <20091105123456.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> It may be wise to forbid a combination of options if it
> encourages mistakes or a wrong workflow, but I don't think
> using --author and --reset-author with 'git commit --amend'
> is such a case.
>
> Imagine somebody other than you (eg. me) were the maintainer,
> and a message by Szeder was sent with a good commit log message.
>
> http://article.gmane.org/gmane.comp.version-control.git/132029
>
> Then you sent a replacement patch that solves the same problem
> in a more elegant way, but without anything that is usable as the
> commit log message.
>
> http://article.gmane.org/gmane.comp.version-control.git/132041
>
> If I were the maintainer, I would find it very convenient if I can
> work like this:
>
> % git am -s 132029 --- first I apply Szeder's version
>
> Then I see your message. Replace the code change but use Szeder's
> log message.
>
> % git reset --hard HEAD^
> % git am 132041 --- your version with no usable log message
> % git commit --amend -s -c @{2} --author='Junio C Hamano <...>'
Thanks.
So you commit Szeder's and then commit mine (make them independent), and
amend the log message of the latter using the message from the former, and
assign the authorship of the latter to the resulting commit?
That is a much more understandable argument than just claiming "--author
should be usable with --reset-author" without clearly stating why that
would help. I think you forgot to add --reset-author to the last command
line, though.
But I think it is showing that --reset-author is actually suboptimal way
to solve your scenario. In the last command in your sequence, you don't
want to add "--reset-author --author=X" but want "--reuse-only-message"
option.
And I think it makes much more sense than the alternative semantics we
came up with during this discussion. --mine (or --reset-author) to
declare that "I am the author" was not what we wanted after all(yes, I am
guilty for suggesting it). What we want is "I am using -C/-c/--amend and
I want to borrow only the message part from the named commit (obviously
"amend" names the HEAD commit implicitly). Determine the authorship
information (including author timestamp) as if I didn't use that option."
^ permalink raw reply
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-05 5:34 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041745x577f7e4rc678da4d7d559193@mail.gmail.com>
On Thu, 5 Nov 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Wed, Nov 4, 2009 at 22:30, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> >> On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> >> > That's not true for "git pull <url> <branch>"; we do want the remote ref,
> >> > but it doesn't have a local peer.
>
> No, I don't think that's right, when doing a fetch we want to store
> the refs somewhere, sure, but not under 'refs/heads/<branch>', perhaps
> 'refs/hg/fetch/<branch>', either way, the current code does not work.
I think you've still got things backwards. From the point of view of
transport.c, refs/<vcs> is entirely opaque, and we never look at it. Those
aren't local peers. They're a way for the helper to communicate to
transport-helper.c. The user says: pull refs/heads/master of this hg
repo. Transport.c tries to fetch refs/heads/master and get the sha1 to
write into FETCH_HEAD or wherever. Transport-helper.c says "import
refs/heads/master", and git-fast-import reads the resulting script and
writes some ref that the helper tells it to write. Then transport-helper.c
figures out where the ref was written, reads it, and updates the struct
ref representing the remote info. Then builtin-fetch looks at the struct
ref and writes it to the local repositories ref namespace or FETCH_HEAD.
> >> >I think going straight to the refspec
> >> > command is the right answer.
> >>
> >> Can you clarity what you mean with "the refspec command"?
> >
> > Whatever it is that lets the helper tell the transport code where in the
> > helper's private namespace to look for refs. I'd been thinking the helper
> > would advertize the "refspec" capability, and the transport code would
> > call the "refspec" command in order to get the helper to report that; but
> > then I actually only said that the helper reports refspec, and not
> > proposed a name for the command.
>
> Currently I'm implementing so that it would work like this for the svn helper:
>
> $ echo capabilities | git remote-svn origin /path/to/hg/repo
> import
> refspec +refs/trunk:refs/svn/origin/trunk
> refspec +refs/branches/*:refs/svn/origin/*
>
> That way we can put the refspec in the config file at clone time.
>
> Now I've been browsing through the builtin-fetch code, and it looks
> like the main problem is going to be to apply this refspec at all.
> I'll have a more extensive look tomorrow.
This is entirely not what I think we should have. The config file should
say refs/heads/*:refs/remotes/origin/* like it always does, because the
transport will list the refs like refs/heads/* and refs/tags/* and return
them like that.
I'll see if I can write up an untested patch that does what I'm thinking
of.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-05 5:24 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <d411cc4a0911032158j2a4664e5w2601c4af59ba0837@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> The technical documentation for the packfile protocol is both sparse and
> incorrect. This documents the fetch-pack/upload-pack and send-pack/
> receive-pack protocols much more fully.
Thanks.
In this round, my comments ended up being mostly "nit-picky", although I
spotted a few logic errors.
> +Git Transport
> +-------------
> +
> +The Git protocol starts off by sending the command and repository
"transport"
> +on the wire using the pkt-line format, followed by a null byte and a
> +hostname paramater, terminated by a null byte.
The name of the byte whose value is 0 is "NUL", not "NULL" (as you
correctly did in the "Reference Discovery" section).
> +Basically what the Git client is doing to connect to an 'upload-pack'
> ...
> +SSH Transport
> ...
> +It is basically equivalent to running this:
> +Reference Discovery
> +-------------------
> +
> +When the client initially connects the server will immediately respond
> +with a listing of each reference it has (all branches and tags) along
> +with the object name that each reference currently points to.
> +
> + $ echo -e -n "0039git-upload-pack
> /schacon/gitbook.git\0host=example.com\0" |
Oops...
> + nc -v example.com 9418
> + 00887217a7c7e582c46cec22a130adf4b9d7d950fba0 HEAD\0multi_ack
> thin-pack side-band side-band-64k ofs-delta shallow no-progress
> include-tag
Oops...
It is perfectly fine to fold a long line in the document. What I
earlier suggested was to state the fact that the document _did_ fold
a long line for typographical reasons and in the actual protocol it
is a single long line to the reader.
> +The returned response is a pkt-line stream describing each ref and
> +its known value. The stream MUST be sorted by name according to
> +the C locale ordering.
Do we need to say "known"?
> ...
> + advertised-refs = (no-refs / list-of-refs)
> + flush-pkt
> +
> + no-refs = PKT-LINE(zero-id SP "capabilities^{}"
> + NUL capability-list LF)
> +
> + list-of-refs = first-ref *other-ref
> + first-ref = PKT-LINE(obj-id SP refname
> + NUL capability-list LF)
> +
> + other-ref = PKT-LINE(other-tip / other-peeled)
> + other-tip = obj-id SP refname LF
> + other-peeled = obj-id SP refname "^{}" LF
> +
> + capability-list = capability *(SP capability)
> + capability = 1*(LC_ALPHA / DIGIT / "-" / "_")
> + LC_ALPHA = %x61-7A
> +----
> +
> +Server and client SHOULD use lowercase for SHA1, both MUST treat SHA1
> +as case-insensitive.
I hate to sound like a broken parrot, but ...
- You do not have SHA1 in the above list; say "obj-id".
- Why is this SHOULD and not MUST? It's not like you or somebody else
have already written such a broken server or client and you have to
grandfather it by defining what goes over the wire looser than the
current practice. I think this is taking the "be liberal in what you
accept" mantra too literally without real reason for doing so.
> +Packfile Negotiation
> +--------------------
> +After reference and capabilities discovery, the client can decide
> +to terminate the connection by sending a flush-pkt, telling the
> +server it can now gracefully terminate (as happens with the ls-remote
> +command) or it can enter the negotiation phase, where the client and
> +server determine what the minimal packfile necessary for transport is.
> +
> +Once the client has the initial list of references that the server
> +has, as well as the list of capabilities, it will begin telling the
> +server what objects it wants and what objects it has, so the server
> +can make a packfile that only has the objects that the client needs.
"only has" -> "only contains", perhaps, as the above has too many has
already?
> +The client will also send a list of the capabilities it supports out
> +of what the server said it could do with the first 'want' line.
It is not _wrong_ per-se, but it is not about client "supporting". It is
asking these particular protocol extensions enabled. How about...
.. will also send a list of the capabilities it wants to be in
effect, out of ...
> +If the server reads 'have' lines, it then will respond by ACKing any
> +of the obj-ids the client said it had that the server also has. The
> +server will ACK obj-ids differently depending on which ack mode is
> +signaled by the client.
Perhaps "signaled" -> "chosen"?
> +In multi_ack mode:
> ...
> +In multi_ack_detailed mode:
> ...
> +Without multi_ack:
> ...
The last one is "without multi_ack or multi_ack_detailed", isn't it?
I think the order you described these three is sensible (from the most
commonly deployed to merely describing a historical practice).
> +Packfile Data
> +-------------
> +
> +Now that the client and server have done some negotiation about what
> +the minimal amount of data that can be sent to the client is, the server
> +will construct and send the required data in packfile format.
Perhaps "done some" -> "finished"?
Is it "can be sent" or "needs to be sent"?
> +Pushing Data To a Server
> ...
> +references to be complete. Once all the data is received and validated,
> +the server will then update it's references to what the client specified.
"it's" -> "its".
> +Reference Discovery
> +-------------------
> +
> +The reference discovery phase is done nearly the same way as it is in the
> +fetching protocol. Each reference obj-id and name on the server is sent
> +in packet-line format to the client, followed by a flush packet. The only
I slightly prefer "a flush packet" spelled out like this, but please be
consistent either way. You have only two instances of spelled-out "flush
packet" around this section, and everywhere else in this patch you used
"flush-pkt" (I am talking about explanation prose, not ABNF, in which you
consistently used "flush-pkt" everywhere), so changing these "flush
packet" to "flush-pkt" may be easier to change and less error prone.
> +A pack-file MUST be sent if either create or update command is used,
> +even if the server already has all the necessary objects. In this
> +case the client MUST send an empty pack-file. The only time this
> +is likely to happen is if the client is doing something like creating
> +a new branch that points to an existing obj-id.
"doing something like" is redundant.
> +The server will receive the packfile, unpack it, then validate each
> +reference that is being updated that it hasn't changed while the request
> +was being processed (the obj-id is still the same as the old-id), and
> +it will run any update hooks to make sure that the update is acceptable.
> +If all of that is fine, the server will then update the references.
Strictly speaking, "unpack it" is a wrong thing to say in a protocol
specification document. The only requirement is for the server to make
objects in it accessible before the ref update takes place. The objects
need to be accessible when hooks run.
I was about to suggest "store objects in it in the repository" instead,
but in the status report we do use a successful "unpack" to mean "made
objects accessible successfully", so it probably is Ok as-is. At least I
cannot think of a better way to rephrase this part.
> +Report Status
> +-------------
> +
> +After receiving the pack data from the sender, the client sends a
"the client sends" -> "the server sends".
Even though it makes me a bit uneasy to describe these entities "server vs
client" (I'd prefer "sender" vs "receiver"), the entire document is
written based on the assumption that "clients" fetch from or push to "the
server", so let's be consistent.
> +report if 'report-status' capability was sent to the server.
> +It is a short listing of what happened in that update. It will first
> +list the status of the packfile unpacking as either 'unpack ok' or
> +'unpack [error]'. Then it will list the status for each of the references
> +that it tried to update. Each line be either 'ok [refname]' if the
Sorry for asking a language question but "be"?
> +An example client/server communication might look like this:
> +
> +----
> + S: 007c74730d410fcb6603ace96f1dc55ea6196122532d
> refs/heads/local\0report-status delete-refs ofs-delta\n
> + S: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe refs/heads/debug\n
> + S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/master\n
> + S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
> + S: 0000
> +
> + C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe
> 74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
> + C: 003e74730d410fcb6603ace96f1dc55ea6196122532d
> 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
Oops.
> diff --git a/Documentation/technical/protocol-capabilities.txt
> b/Documentation/technical/protocol-capabilities.txt
> new file mode 100644
> index 0000000..f4bf986
> --- /dev/null
> +++ b/Documentation/technical/protocol-capabilities.txt
> @@ -0,0 +1,186 @@
> +Git Protocol Capabilities
> +=========================
> +
> +Servers SHOULD support all capabilities defined in this document.
> +
> +On the very first line of the initial server response of either
> +receive-pack and upload-pack the first reference is followed by
> +a null byte and then a list of space delimited server capabilities.
"null" -> "NUL"
> +Client will then send a space separated list of capabilities it
> +can support. The client SHOULD NOT ask for capabilities the server
> +did not say it supports.
> +
> +Server MUST ignore capabilities it does not understand. Server MUST
> +NOT ignore capabilities that client requested and server advertised.
> +
I hate to sound like a broken parrot, but ...
- It is more like "Client will ... it wants enabled", not "it can
support".
- Is this really "SHOULD NOT", as opposed to "MUST NOT"? What are
examples of plausible justifications for client implementations to
violate this requirement and ask for a capability that it knows the
server does not support?
- Shouldn't server "MUST diagnose and abort" connection when client
requests a protocol feature (i.e. capability) that it does not
understand?
When the server advertises a capability it itself does not understand
and the client asked for it, then the client can legitimately ask for
it, and the server is not allowed to ignore it, but it must ignore it, so
it cannot satisfy this requirement. We should spell out that the server
MUST NOT advertise capabilities it does not understand.
> +thin-pack
> +---------
> +
> +This capability implies that the server can send 'thin' packs, packs
Does it imply or explicitly state? I think it is the latter.
> +which do not contain base objects; if those base objects are available
> +on client side. Client requests 'thin-pack' capability when it
> +understands how to "thicken" them adding required delta bases making
> +them self contained.
A single pack is sent over the wire and thicking it will create a single
pack self contained, hence "making it self contained".
> +Client MUST NOT request 'thin-pack' capability if it cannot turn thin
> +packs into proper independent packs.
"a think pack into a self contained pack"
> +side-band, side-band-64k
> +------------------------
> +
> +This means that server can send, and client understand multiplexed
> +progress reports and error info interleaved with the packfile itself.
Either "This" -> "This capability", or just begin the sentence with
"Server can send..." like your description of "ofs-delta".
> +delete-refs
> +-----------
> +
> +If the server sends back the 'delete-refs' capability, it means that
> +it is capable of accepting an all-zeroed SHA-1 value as the target
> +value of a reference update. It is not sent back by the client, it
> +simply informs the client that it can be sent zeroed SHA-1 values
> +to delete references.
You earlier called these "all-zeroed SHA-1 value"s "zero-id"s. Be
consistent.
> diff --git a/Documentation/technical/protocol-common.txt
> b/Documentation/technical/protocol-common.txt
> new file mode 100644
> index 0000000..2dca642
> --- /dev/null
> +++ b/Documentation/technical/protocol-common.txt
> @@ -0,0 +1,96 @@
> ...
> +A refname is a hierarichal octet string beginning with "refs/" and
"hierarchical"
^ permalink raw reply
* Re: [RFC/PATCH 0/3] use '--bisect-refs' as bisect rev machinery option
From: Christian Couder @ 2009-11-05 5:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vtyxaez4f.fsf@alter.siamese.dyndns.org>
On Wednesday 04 November 2009, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > So to do that you would use "git bisect start ..." and then you could
> > use:
> >
> > $ git rev-list --bisect HEAD --not $GOOD_COMMITS
> >
> > to get the commit that you would have to test if the current commit is
> > bad and:
> >
> > $ git rev-list --bisect $BAD --not $GOOD_COMMITS HEAD
> >
> > to get the commit that you would have to test if the current commit is
> > good.
>
> Even in that case, the problem is still about narrowing the set of the
> current bisection graph. If --bisect option implicitly grabs good and
> bad defined in the refspace like Linus's patch does, it will give you the
> same behaviour of the above two commands, no?
I think it will probably work when you add a good rev, but in the case where
you give a different bad (the first command above) it does not work the
same.
The test case in patch 1/3 shows that. It does:
git bisect start $HASH7 $HASH1 &&
...
rev_list2=$(git rev-list --bisect $HASH3 --not $HASH1) &&
test "$rev_list2" = "$HASH2"
and that last command fails.
Best regards,
Christian.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox