* [PATCH] git-fast-import(1): remove duplicate "--done" option
From: John Keeping @ 2013-01-05 16:06 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Eric S. Raymond, Sverre Rabbelier
The "--done" option to git-fast-import is documented twice in its manual
page. Combine the best bits of each description, keeping the location
of the instance that was added first.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
I'm guessing that the reason the option was documented again (in commit
3266de10) is because the options are not in an obvious order. There
does seem to be some grouping of the options by type, but without
subheadings I wonder if it would make more sense to just put them all in
alphabetical order?
Documentation/git-fast-import.txt | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 68bca1a..4ef5721 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -39,10 +39,6 @@ OPTIONS
See ``Date Formats'' below for details about which formats
are supported, and their syntax.
--- done::
- Terminate with error if there is no 'done' command at the
- end of the stream.
-
--force::
Force updating modified existing branches, even if doing
so would cause commits to be lost (as the new commit does
@@ -108,7 +104,8 @@ OPTIONS
output.
--done::
- Require a `done` command at the end of the stream.
+ Terminate with error if there is no 'done' command at the
+ end of the stream.
This option might be useful for detecting errors that
cause the frontend to terminate before it has started to
write a stream.
--
1.8.0.2
^ permalink raw reply related
* Re: cvsps, parsecvs, svn2git and the CVS exporter mess
From: Eric S. Raymond @ 2013-01-05 15:58 UTC (permalink / raw)
To: Bart Massey
Cc: Max Horn, Michael Haggerty, Yann Dirson, Heiko Voigt,
Antoine Pelisse, Keith Packard, David Mansfield, git
In-Reply-To: <CAA6gtpky9JxFDdpLM6kY9su-9FWX8RoWHU4uptd_Zk+ZJuhrtA@mail.gmail.com>
Bart Massey <bart@cs.pdx.edu>:
> I don't know what Eric Raymond "officially end-of-life"-ing parsecvs means?
You and Keith handed me the maintainer's baton. If I were to EOL it,
that would be the successor you two designated judging in public that
the code is unsalvageable or has become pointless. If you wanted to
exclude the possibility that a successor would make that call, you
shouldn't have handed it off in a state so broken that I can't even
test it properly.
But I don't in fact think the parsecvs code is pointless. The fact that it
only needs the ,v files is nifty and means it could be used as an RCS
exporter too. The parsing and topo-analysis stages look like really
good work, very crisp and elegant (which is no less than I'd expect
from Keith, actually).
Alas, after wrestling with it I'm beginning to wonder whether the
codebase is salvageable by anyone but Keith himself. The tight coupling
to the git cache mechanism is the biggest problem. So far, I can't
figure out what tree.c is actually doing in enough detail to fix it or pry
it loose - the code is opaque and internal documentation is lacking.
More generally, interfacing to the unstable API of libgit was clearly
a serious mistake, leading directly to the current brokenness. The
tool should have emitted an import stream to begin with. I'm trying
to fix that, but success is looking doubtful.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: cvsps, parsecvs, svn2git and the CVS exporter mess
From: Eric S. Raymond @ 2013-01-05 15:11 UTC (permalink / raw)
To: Max Horn
Cc: Michael Haggerty, Yann Dirson, Heiko Voigt, Antoine Pelisse,
Bart Massey, Keith Packard, David Mansfield, git
In-Reply-To: <1E7F9F86-F040-42E4-98C4-152B8CCE47CE@quendi.de>
Max Horn <postbox@quendi.de>:
> Hm, you snipped this part of Michael's mail:
>
> >> However, if that is a
> >> problem, it is possible to configure cvs2git to write the blobs inline
> >> with the rest of the dumpfile (this mode is supported because "hg
> >> fast-import" doesn't support detached blobs).
>
> I would call "hg fast-import" a main potential customer, given that there "cvs2hg" is another part of the cvs2svn suite. So I can't quite see how you can come to your conclusion above...
Perhaps I was unclear. I consider the interface design error to
be not in the fact that all the blobs are written first or detached,
but rather that the implementation detail of the two separate journal
files is ever exposed.
I understand why the storage of intermediate results was done this
way, in order to decrease the tool's working set during the run, but
finishing by automatically concatenating the results and streaming
them to stdout would surely have been the right thing here.
The downstream cost of letting the journalling implementation be
exposed, instead, can be seen in this snippet from the new git-cvsimport
I've been working on:
def command(self):
"Emit the command implied by all previous options."
return "(cvs2git --username=git-cvsimport --quiet --quiet --blobfile={0} --dumpfile={1} {2} {3} && cat {0} {1} && rm {0} {1})".format(tempfile.mkstemp()[1], tempfile.mkstemp()[1], self.opts, self.modulepath)
According to the documentation, every caller of csv2git must go
through analogous contortions! This is not the Unix way; if Unix
design principles had been minimally applied, that second line would
just read like this:
return "cvs2git --username=git-cvsimport --quiet --quiet"
If Unix design principles had been thoroughly applied, the "--quiet
--quiet" part would be unnecessary too - well-behaved Unix commands
*default* to being completely quiet unless either (a) they have an
exceptional condition to report, or (b) their expected running time is
so long that tasteful silence would leave users in doubt that they're
working.
(And yes, I do think violating these principles is a lapse of taste when
git tools do it, too.)
Michael Haggerty wants me to trust that cvs2git's analysis stage has
been fixed, but I must say that is a more difficult leap of faith when
two of the most visible things about it are still (a) a conspicuous
instance of interface misdesign, and (b) documentation that is careless and
incomplete.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH] fix compilation with NO_PTHREADS
From: Jeff King @ 2013-01-05 14:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Commit 1327452 cleaned up an unused parameter from
wait_or_whine, but forgot to update a caller that is inside
"#ifdef NO_PTHREADS".
Signed-off-by: Jeff King <peff@peff.net>
---
I happened to notice this while looking at the sigpipe topic. I guess
not many people are building with NO_PTHREADS these days.
run-command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index cfb7274..0471219 100644
--- a/run-command.c
+++ b/run-command.c
@@ -725,7 +725,7 @@ int finish_async(struct async *async)
int finish_async(struct async *async)
{
#ifdef NO_PTHREADS
- return wait_or_whine(async->pid, "child process", 0);
+ return wait_or_whine(async->pid, "child process");
#else
void *ret = (void *)(intptr_t)(-1);
--
1.8.1.rc1.16.g6d46841
^ permalink raw reply related
* Re: [BUG] git submodule update is not fail safe
From: Jens Lehmann @ 2013-01-05 14:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, git, Manlio Perillo, W. Trevor King
In-Reply-To: <50E83224.2070701@web.de>
Am 05.01.2013 15:01, schrieb Jens Lehmann:
> Am 04.01.2013 22:51, schrieb Junio C Hamano:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>>
>>> $ git submodule update --init
>>> ...
>>> Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered
>>> for path 'roms/vgabios'
>>> fatal: unable to connect to anongit.freedesktop.org:
>>> anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
>>>
>>> Unable to fetch in submodule path 'pixman'
>>>
>>> $ git submodule update --init
>>> fatal: Needed a single revision
>>> Unable to find current revision in submodule path 'pixman'
>>>
>>> The problem is easy to solve: manually remove the pixman directory;
>>> however IMHO git submodule update should not fail this way since it may
>>> confuse the user.
>>
>> Sounds like a reasonable observation. Jens, Heiko, comments?
>
> The reason seems to be that clone leaves a partial initialized .git
> directory in case of connection problems. The next time submodule
> update runs it tries to revive the submodule from .git/modules/<name>
> but fails as there are no objects in it.
>
> This looks like a bug in clone to me, as it takes precautions to clean
> up if something goes wrong but doesn't do that in this case. But while
> glancing over the code I didn't find out what goes wrong here.
I dug a bit deeper here and found the reason: In remove_junk() of
builtin/clone.c the junk_git_dir points to the gitfile in the
submodules work tree, not the .git/modules/<name> directory where
clone was told to put the git directory. Will see if I can come up
with a patch soonish ...
^ permalink raw reply
* Re: [BUG] git submodule update is not fail safe
From: Manlio Perillo @ 2013-01-05 14:49 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Heiko Voigt, git, W. Trevor King
In-Reply-To: <50E83224.2070701@web.de>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 05/01/2013 15:01, Jens Lehmann ha scritto:
> [...]
>>> $ git submodule update --init
>>> fatal: Needed a single revision
>>> Unable to find current revision in submodule path 'pixman'
>>>
>>> The problem is easy to solve: manually remove the pixman directory;
>>> however IMHO git submodule update should not fail this way since it may
>>> confuse the user.
>>
>> Sounds like a reasonable observation. Jens, Heiko, comments?
>
> The reason seems to be that clone leaves a partial initialized .git
> directory in case of connection problems. The next time submodule
> update runs it tries to revive the submodule from .git/modules/<name>
> but fails as there are no objects in it.
>
> [...]
>
> If this isn't seen as a bug in clone, we could also remove the
> .git/modules/<name> directory in module_clone() of git-submodule.s
> h when the clone fails. Manilo,
Its Manlio, not Manilo ;-).
> does the following patch remove the
> problems you are seeing (after removing .git/modules/pixman manually)?
>
I don't think I can test it right now, since the problem can only be
reproduced when git clone fails due to network problems.
Without the patch, if I remove the .git/modules/pixman directory,
`git submodule update --init pixamp` fails:
Unable to find current revision in submodule path 'pixman'
fatal: Not a git repository: pixman/../.git/modules/pixman
To reproduce the problem, however, it seems all you need to do is to
send SIGINT signal during `git submodule update` :
$ git submodule update --init pixman
Cloning into 'pixman'...
remote: Counting objects: 10137, done.
^C
$ git submodule update pixman
remote: Counting objects: 10137, done.
^C
$ git submodule update pixman
fatal: Needed a single revision
Unable to find current revision in submodule path 'pixman'
Note that I had to send SIGINT two times, in order to corrupt the module.
I suspect your patch does not fix this (since I don't get the "Clone
failed" error message).
I also noted that If I send SIGINT before git starts counting remote
objects, I get a different count number:
$ git submodule update pixman
Cloning into 'pixman'...
^C
$ git submodule update pixman
remote: Counting objects: 9757, done.
^C
$ git submodule update pixman
fatal: Needed a single revision
Unable to find current revision in submodule path 'pixman'
Note that git is reporting 9757 remote objects, instead of 10137.
P.S.:
sorry for the mail I sent today.
It reported the exact same problem I reported yesterday: this morning I
was rather sure that I got a different error message from submodule
update...
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDoPZMACgkQscQJ24LbaUTfNQCdFvhSQwGlJZlvOr+TIHHyDFJY
d8AAn0zuHKjBGIcqr8RH/rftHjomvPtM
=48RN
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH] run-command: encode signal death as a positive integer
From: Jeff King @ 2013-01-05 14:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Bart Trojanowski
In-Reply-To: <20130105140316.GA7272@sigill.intra.peff.net>
On Sat, Jan 05, 2013 at 09:03:16AM -0500, Jeff King wrote:
> In fact, I really wonder if this code from wait_or_whine is actually
> correct:
>
> code = WTERMSIG(status);
> /*
> * This return value is chosen so that code & 0xff
> * mimics the exit code that a POSIX shell would report for
> * a program that died from this signal.
> */
> code -= 128;
After looking at it some more, it is correct, but I think we could make
life slightly easier for callers. See the patch below. I've tried to
re-state the somewhat rambling argument from my previous email;
hopefully it makes sense.
-- >8 --
Subject: [PATCH] run-command: encode signal death as a positive integer
When a sub-command dies due to a signal, we encode the
signal number into the numeric exit status as "signal -
128". This is easy to identify (versus a regular positive
error code), and when cast to an unsigned integer (e.g., by
feeding it to exit), matches what a POSIX shell would return
when reporting a signal death in $? or through its own exit
code.
So we have a negative value inside the code, but once it
passes across an exit() barrier, it looks positive (and any
code we receive from a sub-shell will have the positive
form). E.g., death by SIGPIPE (signal 13) will look like
-115 to us in inside git, but will end up as 141 when we
call exit() with it. And a program killed by SIGPIPE but run
via the shell will come to us with an exit code of 141.
Unfortunately, this means that when the "use_shell" option
is set, we need to be on the lookout for _both_ forms. We
might or might not have actually invoked the shell (because
we optimize out some useless shell calls). If we didn't invoke
the shell, we will will see the sub-process's signal death
directly, and run-command converts it into a negative value.
But if we did invoke the shell, we will see the shell's
128+signal exit status. To be thorough, we would need to
check both, or cast the value to an unsigned char (after
checking that it is not -1, which is a magic error value).
Fortunately, most callsites do not care at all whether the
exit was from a code or from a signal; they merely check for
a non-zero status, and sometimes propagate the error via
exit(). But for the callers that do care, we can make life
slightly easier by just using the consistent positive form.
This actually fixes two minor bugs:
1. In launch_editor, we check whether the editor died from
SIGINT or SIGQUIT. But we checked only the negative
form, meaning that we would fail to notice a signal
death exit code which was propagated through the shell.
2. In handle_alias, we assume that a negative return value
from run_command means that errno tells us something
interesting (like a fork failure, or ENOENT).
Otherwise, we simply propagate the exit code. Negative
signal death codes confuse us, and we print a useless
"unable to run alias 'foo': Success" message. By
encoding signal deaths using the positive form, the
existing code just propagates it as it would a normal
non-zero exit code.
The downside is that callers of run_command can no longer
differentiate between a signal received directly by the
sub-process, and one propagated. However, no caller
currently cares, and since we already optimize out some
calls to the shell under the hood, that distinction is not
something that should be relied upon by callers.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-run-command.txt | 6 ++----
editor.c | 2 +-
run-command.c | 2 +-
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
index f18b4f4..5d7d7f2 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -55,10 +55,8 @@ The functions above do the following:
non-zero.
. If the program terminated due to a signal, then the return value is the
- signal number - 128, ie. it is negative and so indicates an unusual
- condition; a diagnostic is printed. This return value can be passed to
- exit(2), which will report the same code to the parent process that a
- POSIX shell's $? would report for a program that died from the signal.
+ signal number + 128, ie. the same value that a POSIX shell's $? would
+ report. A diagnostic is printed.
`start_async`::
diff --git a/editor.c b/editor.c
index 065a7ab..27bdecd 100644
--- a/editor.c
+++ b/editor.c
@@ -51,7 +51,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
sigchain_push(SIGINT, SIG_IGN);
sigchain_push(SIGQUIT, SIG_IGN);
ret = finish_command(&p);
- sig = ret + 128;
+ sig = ret - 128;
sigchain_pop(SIGINT);
sigchain_pop(SIGQUIT);
if (sig == SIGINT || sig == SIGQUIT)
diff --git a/run-command.c b/run-command.c
index 757f263..cfb7274 100644
--- a/run-command.c
+++ b/run-command.c
@@ -249,7 +249,7 @@ static int wait_or_whine(pid_t pid, const char *argv0)
* mimics the exit code that a POSIX shell would report for
* a program that died from this signal.
*/
- code -= 128;
+ code += 128;
} else if (WIFEXITED(status)) {
code = WEXITSTATUS(status);
/*
--
1.8.1.rc1.16.g6d46841
^ permalink raw reply related
* Re: [BUG] git submodule update is not fail safe
From: Jens Lehmann @ 2013-01-05 14:07 UTC (permalink / raw)
To: Manlio Perillo; +Cc: Junio C Hamano, Heiko Voigt, git, W. Trevor King
In-Reply-To: <50E83001.9000505@gmail.com>
Am 05.01.2013 14:52, schrieb Manlio Perillo:
> Il 04/01/2013 22:51, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>>> $ git submodule update --init
>>> ...
>>> Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered
>>> for path 'roms/vgabios'
>>> fatal: unable to connect to anongit.freedesktop.org:
>>> anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
>>>
>>> Unable to fetch in submodule path 'pixman'
>>>
>>> $ git submodule update --init
>>> fatal: Needed a single revision
>>> Unable to find current revision in submodule path 'pixman'
>>>
>>> The problem is easy to solve: manually remove the pixman directory;
>>> however IMHO git submodule update should not fail this way since it may
>>> confuse the user.
>
>> Sounds like a reasonable observation. Jens, Heiko, comments?
>
> I have found another, related problem.
>
> Today I tried to update qemu submodules again, however the command
> failed with an "obscure" error message:
>
> $ git submodule update pixman
> fatal: Needed a single revision
> Unable to find current revision in submodule path 'pixman'
>
>
> The pixman submodule is the one that I failed to update in the very begin.
> The problem is not with the pixman or qemu repository: if I clone again
> qemu (with the --recursive option), all is ok.
>
> The problem is with the private working copy (in .git/modules/pixman)
> being corrupted:
>
> $git log
> fatal: bad default revision 'HEAD'.
>
> The HEAD file contains "ref: refs/heads/master", but the refs/heads
> directory is empty.
Yep, as I explained in my other email the partially set up
.git/modules/pixman is the reason for the trouble you have.
> By the way: since git submodule is a porcelain command, IMHO it should
> not show to the user these low level error message; at least it should
> give more details.
> As an example, in this case it could say something like:
>
> the local module "pixmap" seems to be corrupted.
> Run xxx to remove the module and yyy to create it again.
>
> The ideal solution is, for submodule update, to never leave an
> incomplete directory; that is: the update command should be atomic.
I agree that submodule update should not leave an inconsistent state.
In that case you wouldn't see any low level error messages (which I
think is ok if something the porcelain didn't expect to happen occurs,
like it did here).
^ permalink raw reply
* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Anand Kumria @ 2013-01-05 14:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Mackerras, git
In-Reply-To: <7vhamwse2c.fsf@alter.siamese.dyndns.org>
Hi Junio,
On 4 January 2013 23:50, Junio C Hamano <gitster@pobox.com> wrote:
> Anand Kumria <wildfire@progsoc.org> writes:
>
>> By selecting a tag within gitk you can display information about it.
>> This information is output by using the command
>>
>> 'git cat-file tag <tagid>'
>>
>> This outputs the *raw* information from the tag, amongst which is the
>> time - in seconds since the epoch. As useful as that value is, I find it
>> a lot easier to read and process time which it is something like:
>>
>> "Mon Dec 31 14:26:11 2012 -0800"
>>
>> This change will modify the display of tags in gitk like so:
>>
>> @@ -1,7 +1,7 @@
>> object 5d417842efeafb6e109db7574196901c4e95d273
>> type commit
>> tag v1.8.1
>> -tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
>> +tagger Junio C Hamano <gitster@pobox.com> Mon Dec 31 14:26:11 2012 -0800
>>
>> Git 1.8.1
>> -----BEGIN PGP SIGNATURE-----
>>
>> Signed-off-by: Anand Kumria <wildfire@progsoc.org>
>> ---
>
> Sounds like a sensible thing to do but I didn't check how else
> (other than purely for displaying) this string is used.
As far as I can tell it is only used for display (cached_tagcontent in
gitk) purposes.
> Paul, the patch is not made against your tree, so if you choose to
> take it you would need to strip the leading directory at the top.
Sorry, I didn't know that gitk had been split back out (and
Documentation/gitk.txt still mentions it is part of the git suite).
Regards,
Anand
^ permalink raw reply
* Re: [RFC/PATCH] avoid SIGPIPE warnings for aliases
From: Jeff King @ 2013-01-05 14:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bart Trojanowski
In-Reply-To: <7vsj6gsi7v.fsf@alter.siamese.dyndns.org>
On Fri, Jan 04, 2013 at 02:20:52PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I have two reservations with this patch:
> >
> > 1. We are ignoring SIGPIPE all the time. For an alias that is calling
> > "log", that is fine. But if pack-objects dies on the server side,
> > seeing that it died from SIGPIPE is useful data, and we are
> > squelching that. Maybe callers of run-command should have to pass
> > an "ignore SIGPIPE" flag?
>
> What should this do:
>
> GIT_PAGER='head -n 1' git -p -c alias.o='!cat longfile' o
>
> Should it behave just like
>
> cat longfile | head -n 1
>
> or should it behave differently?
With respect to error messages, I'd think they should behave the same.
But they don't necessarily. The latter does not print any message at
all. But consider this version of the former:
$ cat foo
#!/bin/sh
exec cat longfile
$ git -c alias.o='!./foo' o | head -n 1
error: ./foo died of signal 13
fatal: While expanding alias 'o': './foo': Success
So why don't we see that message more often? There are two reasons.
One reason is that we piped it ourselves here. When git pipes to the
pager, it sends stderr along the same channel. So even if you did:
GIT_PAGER='head -n 1' git -p -c alias.o='!./foo' o
git writes the error, but it goes to the pager, which has already ended
(since that is what caused the SIGPIPE in the first place). But imagine
that your sub-command is actually invoking git itself, and it is the
sub-git which starts the pager. Meaning the outer git wrapper's stderr
is _not_ redirected. Like this:
$ cat foo
#!/bin/sh
exec git log -p
$ GIT_PAGER='head -n 1' git -c alias.o='!./foo' o
error: ./foo died of signal 13
fatal: While expanding alias 'o': './foo': Success
The second reason is that most shells will "eat" the SIGPIPE exit
status, and convert it into a high, positive error code. You can see
that effect here:
$ GIT_PAGER='head -n 1' git log -p
$ echo $?
141
And since we execute aliases via the shell, we end up seeing the
converted exit code (141) instead of the signal death. _Unless_ we
optimize out the shell call (which is why we see it by putting the
command inside "./foo", which git executes directly, but not when we
give the literal "cat longfile", which git will feed to the shell).
Or at least that's _one_ way to see it. Another way is to use a shell
that does not do such conversion. Setting SHELL_PATH to zsh seems to do
so, and I think that is how Bart ran into it (my patch is a followup to
a Google+ posting he made).
> I am having a feeling that whatever external command given as the
> value of alias.$cmd should choose what error status it wants to be
> reported.
I suppose. It means that our "do not run the shell if there are no
meta-characters" optimization is leaky, since the exit code behaves
differently depending on whether we run the shell (and depending on your
exact shell). One solution would be to fix that leakiness, and if
use_shell is in effect for run-command, to convert a signal death into
the value that the shell would otherwise give us.
In fact, I really wonder if this code from wait_or_whine is actually
correct:
code = WTERMSIG(status);
/*
* This return value is chosen so that code & 0xff
* mimics the exit code that a POSIX shell would report for
* a program that died from this signal.
*/
code -= 128;
If we get signal 13, we end up with -115, because "code" is signed. When
the lower 8 bits are taken, and then converted into an unsigned value,
we get 141: the shell value.
But do we really want to return a negative value here? Should this
instead be:
code += 128
which yields the same code when fed to exit, but internally looks like
the shell version to us? So we get a consistent result whether the shell
was actually used or not.
That makes more sense to me, and would mean that whether we converted
the signal number or whether it was done by a subshell, it looks the
same to us. Callers which care about signals (e.g., the recent changes
to launch_editor to detect SIGINT) would have to be adjusted. But I
think it fixes an obscure bug there. Right now launch_editor is actually
checking the whether the _shell_ died from a signal, and will fail to
notice when an editor invoked by the shell is killed by those signals
(this would be pretty rare, though, because typically SIGINT is
delivered to the shell as well as the editor).
This would also fix the code in handle_alias. It looks for a negative
return code from run_command as the sign that there was an internal
error running the command, and that errno would be valid. But right now
a negative return can also come from signal death.
> > 2. The die_errno in handle_alias is definitely wrong. Even if we want
> > to print a message for signal death, showing errno is bogus unless
> > the return value was -1. But is it the right thing to just pass the
> > negative value straight to exit()? It works, but it is depending on
> > the fact that (unsigned char)(ret & 0xff) behaves in a certain way
> > (i.e., that we are on a twos-complement platform, and -13 becomes
> > 141).
>
> Isn't that what POSIX.1 guarantees us, though?
>
> The value of status may be 0, EXIT_SUCCESS, EXIT_FAILURE, or any
> other value, though only the least significant 8 bits (that is,
> status & 0377) shall be available to a waiting parent process.
Sort of. I was worried about:
1. Not-quite-POSIX platforms (i.e., Windows). But JSixt has said that
is fine, because we already have a compatibility wrapper which
masks off only the low byte.
2. We are relying on the specifics of how a negative value is treated
by exit(). The cast I gave above is guaranteed to work in standard
C, but we do not know the implementation details of exit(). Still,
I think that is being overly paranoid. Any sane implementation will
do what we expect.
-Peff
^ permalink raw reply
* Re: [BUG] git submodule update is not fail safe
From: Jens Lehmann @ 2013-01-05 14:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, git, Manlio Perillo, W. Trevor King
In-Reply-To: <7vzk0osjli.fsf@alter.siamese.dyndns.org>
Am 04.01.2013 22:51, schrieb Junio C Hamano:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>> $ git submodule update --init
>> ...
>> Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered
>> for path 'roms/vgabios'
>> fatal: unable to connect to anongit.freedesktop.org:
>> anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
>>
>> Unable to fetch in submodule path 'pixman'
>>
>> $ git submodule update --init
>> fatal: Needed a single revision
>> Unable to find current revision in submodule path 'pixman'
>>
>> The problem is easy to solve: manually remove the pixman directory;
>> however IMHO git submodule update should not fail this way since it may
>> confuse the user.
>
> Sounds like a reasonable observation. Jens, Heiko, comments?
The reason seems to be that clone leaves a partial initialized .git
directory in case of connection problems. The next time submodule
update runs it tries to revive the submodule from .git/modules/<name>
but fails as there are no objects in it.
This looks like a bug in clone to me, as it takes precautions to clean
up if something goes wrong but doesn't do that in this case. But while
glancing over the code I didn't find out what goes wrong here.
If this isn't seen as a bug in clone, we could also remove the
.git/modules/<name> directory in module_clone() of git-submodule.s
h when the clone fails. Manilo, does the following patch remove the
problems you are seeing (after removing .git/modules/pixman manually)?
diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..4098702 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -208,7 +208,10 @@ module_clone()
git clone $quiet -n ${reference:+"$reference"} \
--separate-git-dir "$gitdir" "$url" "$sm_path"
) ||
- die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
+ (
+ rm -rf "$gitdir" &&
+ die "$(eval_gettext "Clone of '\$url' into submodule path '\$sm_path' failed")"
+ )
fi
# We already are at the root of the work tree but cd_to_toplevel will
^ permalink raw reply related
* Re: [BUG] git submodule update is not fail safe
From: Manlio Perillo @ 2013-01-05 13:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jens Lehmann, Heiko Voigt, git, W. Trevor King
In-Reply-To: <7vzk0osjli.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 04/01/2013 22:51, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>> $ git submodule update --init
>> ...
>> Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered
>> for path 'roms/vgabios'
>> fatal: unable to connect to anongit.freedesktop.org:
>> anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
>>
>> Unable to fetch in submodule path 'pixman'
>>
>> $ git submodule update --init
>> fatal: Needed a single revision
>> Unable to find current revision in submodule path 'pixman'
>>
>> The problem is easy to solve: manually remove the pixman directory;
>> however IMHO git submodule update should not fail this way since it may
>> confuse the user.
>
> Sounds like a reasonable observation. Jens, Heiko, comments?
I have found another, related problem.
Today I tried to update qemu submodules again, however the command
failed with an "obscure" error message:
$ git submodule update pixman
fatal: Needed a single revision
Unable to find current revision in submodule path 'pixman'
The pixman submodule is the one that I failed to update in the very begin.
The problem is not with the pixman or qemu repository: if I clone again
qemu (with the --recursive option), all is ok.
The problem is with the private working copy (in .git/modules/pixman)
being corrupted:
$git log
fatal: bad default revision 'HEAD'.
The HEAD file contains "ref: refs/heads/master", but the refs/heads
directory is empty.
By the way: since git submodule is a porcelain command, IMHO it should
not show to the user these low level error message; at least it should
give more details.
As an example, in this case it could say something like:
the local module "pixmap" seems to be corrupted.
Run xxx to remove the module and yyy to create it again.
The ideal solution is, for submodule update, to never leave an
incomplete directory; that is: the update command should be atomic.
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDoMAEACgkQscQJ24LbaUQVugCggdl36Hx5JIW/hd1SVXWv+ths
zpYAnR+93BfDLaFhXEiaQvu/TickmDA0
=2Mnw
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] Add getenv.so for catching invalid getenv() use via LD_PRELOAD
From: Duy Nguyen @ 2013-01-05 11:37 UTC (permalink / raw)
To: Matt Kraai
Cc: Nguyễn Thái Ngọc, git, Junio C Hamano,
David Michael
In-Reply-To: <20130105103900.GA4200@ftbfs.org>
On Sat, Jan 5, 2013 at 5:39 PM, Matt Kraai <kraai@ftbfs.org> wrote:
> On Sat, Jan 05, 2013 at 03:55:46PM +0700, Nguyễn Thái Ngọc Duy wrote:
>> Perhaps this will help the getenv bug hunting (I assume we do the
>> hunting on Linux platform only). So far it catches this and is stuck
>> at getenv in git_pager().
>
> It seems like a static analysis tool might be able to detect these
> problems. Is there a way to do so using sparse?
That was my first thought. But this may involve flow analysis and I
don't think sparse is up to it. ccc-analyzer is still pretty basic.
And between static analysis and runtime check, I prefer the latter as
it's more reliable as long as you have a good coverage test.
>
>> + n = backtrace(buffer, 100);
>> + symbols = backtrace_symbols(buffer, n);
>> + if (symbols) {
>> + for (i = 0;i < n; i++)
>
> s/;i/; i/
Thanks. I will fix it later if people actually want this.
--
Duy
^ permalink raw reply
* Re: t7061: comments and one failure
From: Antoine Pelisse @ 2013-01-05 11:29 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, Git Mailing List
In-Reply-To: <20130105112432.GA14666@sigill.intra.peff.net>
> Yeah, I can reproduce the problem here on my OS X test box. It seems to
> be related to core.ignorecase. If you put
>
> git config core.ignorecase false
>
> at the top of t7061, it passes on OS X. Likewise, if you set it to true,
> it will start failing on Linux.
Great, so I have a chance to reproduce and fix it. (and it gives me a
massive hint on where the bug can stand).
^ permalink raw reply
* Re: t7061: comments and one failure
From: Jeff King @ 2013-01-05 11:24 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: apelisse, Git Mailing List
In-Reply-To: <50E8096C.7000501@web.de>
On Sat, Jan 05, 2013 at 12:07:24PM +0100, Torsten Bögershausen wrote:
> TC 9 is failing (Mac OS X 10.6),
Yeah, I can reproduce the problem here on my OS X test box. It seems to
be related to core.ignorecase. If you put
git config core.ignorecase false
at the top of t7061, it passes on OS X. Likewise, if you set it to true,
it will start failing on Linux.
So it looks like a real bug, not a test-portability issue.
> Looking into the code, there are 2 questions:
>
> 1) echo "ignored" >.gitignore &&
> We don't need the quoting of a simple string which does not have space in it.
No, but it does not hurt anything.
> 2) : >untracked/ignored &&
> Do we need the colon here?
No, but it does not hurt anything.
-Peff
^ permalink raw reply
* t7061: comments and one failure
From: Torsten Bögershausen @ 2013-01-05 11:07 UTC (permalink / raw)
To: apelisse, Git Mailing List; +Cc: Torsten Bögershausen
Hej,
TC 9 is failing (Mac OS X 10.6),
==========================
expecting success:
>tracked/uncommitted &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
--- expected 2013-01-05 11:01:00.000000000 +0000
+++ actual 2013-01-05 11:01:00.000000000 +0000
@@ -1,4 +1,3 @@
?? .gitignore
?? actual
?? expected
-!! tracked/
not ok 9 - status ignored tracked directory and uncommitted file with --ignore
#
# >tracked/uncommitted &&
# git status --porcelain --ignored >actual &&
# test_cmp expected actual
#
=======================
I haven't been able to dig further into this,
(I can volonteer to do some more debugging).
Looking into the code, there are 2 questions:
1) echo "ignored" >.gitignore &&
We don't need the quoting of a simple string which does not have space in it.
2) : >untracked/ignored &&
Do we need the colon here?
Would it make sence to do the following:
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 0da1214..761a2e7 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -12,10 +12,10 @@ cat >expected <<\EOF
EOF
test_expect_success 'status untracked directory with --ignored' '
- echo "ignored" >.gitignore &&
+ echo ignored >.gitignore &&
mkdir untracked &&
- : >untracked/ignored &&
- : >untracked/uncommitted &&
+ >untracked/ignored &&
+ >untracked/uncommitted &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
'
@@ -43,7 +43,7 @@ EOF
test_expect_success 'status ignored directory with --ignore' '
rm -rf untracked &&
mkdir ignored &&
- : >ignored/uncommitted &&
+ >ignored/uncommitted &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
'
@@ -71,8 +71,8 @@ test_expect_success 'status untracked directory with ignored files with --ignore
rm -rf ignored &&
mkdir untracked-ignored &&
mkdir untracked-ignored/test &&
- : >untracked-ignored/ignored &&
- : >untracked-ignored/test/ignored &&
+ >untracked-ignored/ignored &&
+ >untracked-ignored/test/ignored &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
'
@@ -99,10 +99,10 @@ EOF
test_expect_success 'status ignored tracked directory with --ignore' '
rm -rf untracked-ignored &&
mkdir tracked &&
- : >tracked/committed &&
+ >tracked/committed &&
git add tracked/committed &&
git commit -m. &&
- echo "tracked" >.gitignore &&
+ echo tracked >.gitignore &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
'
@@ -126,7 +126,7 @@ cat >expected <<\EOF
EOF
test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '
- : >tracked/uncommitted &&
+ >tracked/uncommitted &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
'
/Torsten
^ permalink raw reply related
* Re: [PATCH] Add getenv.so for catching invalid getenv() use via LD_PRELOAD
From: Matt Kraai @ 2013-01-05 10:39 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, David Michael
In-Reply-To: <1357376146-7155-1-git-send-email-pclouds@gmail.com>
On Sat, Jan 05, 2013 at 03:55:46PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Perhaps this will help the getenv bug hunting (I assume we do the
> hunting on Linux platform only). So far it catches this and is stuck
> at getenv in git_pager().
It seems like a static analysis tool might be able to detect these
problems. Is there a way to do so using sparse?
> + n = backtrace(buffer, 100);
> + symbols = backtrace_symbols(buffer, n);
> + if (symbols) {
> + for (i = 0;i < n; i++)
s/;i/; i/
--
Matt Kraai
https://ftbfs.org/kraai
^ permalink raw reply
* [PATCH] Add getenv.so for catching invalid getenv() use via LD_PRELOAD
From: Nguyễn Thái Ngọc Duy @ 2013-01-05 8:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Michael,
Nguyễn Thái Ngọc Duy
In-Reply-To: <CAEvUa7niTJVfp8_kuWs50kvhfZ59F-yAuAmeOXEduHXOq-tRFA@mail.gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Perhaps this will help the getenv bug hunting (I assume we do the
hunting on Linux platform only). So far it catches this and is stuck
at getenv in git_pager().
diff --git a/exec_cmd.c b/exec_cmd.c
index 125fa6f..d8be5ce 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -97,7 +97,7 @@ static void add_path(struct strbuf *out, const char *path)
void setup_path(void)
{
- const char *old_path = getenv("PATH");
+ char *old_path = xstrdup(getenv("PATH"));
struct strbuf new_path = STRBUF_INIT;
add_path(&new_path, git_exec_path());
@@ -110,6 +110,7 @@ void setup_path(void)
setenv("PATH", new_path.buf, 1);
+ free(old_path);
strbuf_release(&new_path);
}
contrib/getenv/Makefile | 2 ++
contrib/getenv/getenv.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+)
create mode 100644 contrib/getenv/Makefile
create mode 100644 contrib/getenv/getenv.c
diff --git a/contrib/getenv/Makefile b/contrib/getenv/Makefile
new file mode 100644
index 0000000..4881b85
--- /dev/null
+++ b/contrib/getenv/Makefile
@@ -0,0 +1,2 @@
+getenv.so: getenv.c
+ $(CC) -g -shared -fPIC -ldl -o $@ $<
diff --git a/contrib/getenv/getenv.c b/contrib/getenv/getenv.c
new file mode 100644
index 0000000..e351e10
--- /dev/null
+++ b/contrib/getenv/getenv.c
@@ -0,0 +1,67 @@
+#include <gnu/lib-names.h>
+#include <sys/mman.h>
+#include <dlfcn.h>
+#include <execinfo.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+
+/* Global symbols for easy access from gdb */
+static char *getenv_current;
+static char *getenv_prev;
+
+/*
+ * Intercept standard getenv() via LD_PRELOAD. The return value is
+ * made inaccessible by the next getenv() call. This helps catch
+ * places that ignore the statement "The string pointed to may be
+ * overwritten by a subsequent call to getenv()" [1].
+ *
+ * The backtrace is appended after the env string, which may be
+ * helpful to identify where this getenv() is called in a core dump.
+ *
+ * [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
+ */
+char *getenv(const char *name)
+{
+ static char *(*libc_getenv)(const char*);
+ char *value;
+
+ if (!libc_getenv) {
+ void *libc = dlopen(LIBC_SO, RTLD_LAZY);
+ libc_getenv = dlsym(libc, "getenv");
+ }
+ if (getenv_current) {
+ mprotect(getenv_current, strlen(getenv_current) + 1, PROT_NONE);
+ getenv_prev = getenv_current;
+ getenv_current = NULL;
+ }
+
+ value = libc_getenv(name);
+ if (value) {
+ int len = strlen(value) + 1;
+ int backtrace_len = 0;
+ void *buffer[100];
+ char **symbols;
+ int i, n;
+
+ n = backtrace(buffer, 100);
+ symbols = backtrace_symbols(buffer, n);
+ if (symbols) {
+ for (i = 0;i < n; i++)
+ backtrace_len += strlen(symbols[i]) + 1; /* \n */
+ backtrace_len++; /* NULL */
+ }
+
+ getenv_current = mmap(NULL, len + backtrace_len, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+ memcpy(getenv_current, value, len);
+ value = getenv_current;
+
+ if (symbols) {
+ char *p = getenv_current + len;
+ for (i = 0; i < n; i++)
+ p += sprintf(p, "%s\n", symbols[i]);
+ }
+ }
+ return value;
+}
--
1.8.0.rc2.23.g1fb49df
^ permalink raw reply related
* Re: cvsps, parsecvs, svn2git and the CVS exporter mess
From: Max Horn @ 2013-01-05 8:27 UTC (permalink / raw)
To: esr
Cc: Michael Haggerty, Yann Dirson, Heiko Voigt, Antoine Pelisse,
Bart Massey, Keith Packard, David Mansfield, git
In-Reply-To: <20130103205301.GD26201@thyrsus.com>
On 03.01.2013, at 21:53, Eric S. Raymond wrote:
> Michael Haggerty <mhagger@alum.mit.edu>:
>> There are two good reasons that the output is written to two separate files:
>
> Those are good reasons to write to a pair of tempfiles, and I was able
> to deduce in advance most of what your explanation would be from the
> bare fact that you did it that way.
>
> They are *not* good reasons for having an interface that exposes this
> implementation detail to the caller - that choice I consider a failure
> of interface-design judgment. But I know how to fix this in a simple and
> backward-compatible way, and will do so when I have time to write you
> a patch. Next week or the week after, most likely.
>
> Also, the cvs2git manual page is still rather half-baked and careless,
> with several fossil references to cvs2svn that shouldn't be there and
> obviously incomplete feature coverage. Fixing these bugs is also on my
> to-do list for sometime this month.
>
> I'd be willing to put in this work anyway, but it still in the back of
> my mind that if cvs2git wins the test-suite competition I might
> officially end-of-life both cvsps and parsecvs. One of the features
> of the new git-cvsimport is direct support for using cvs2git as a
> conversion engine.
>
>> A potentially bigger problem is that if you want to handle such
>> blob/dump output, you have to deal with git-fast-import format's "blob"
>> command as opposed to only handling inline blobs.
>
> Not a problem. All of the main potential consumers for this output,
> including reposurgeon, handle the blob command just fine.
Hm, you snipped this part of Michael's mail:
>> However, if that is a
>> problem, it is possible to configure cvs2git to write the blobs inline
>> with the rest of the dumpfile (this mode is supported because "hg
>> fast-import" doesn't support detached blobs).
I would call "hg fast-import" a main potential customer, given that there "cvs2hg" is another part of the cvs2svn suite. So I can't quite see how you can come to your conclusion above...
Cheers,
Max
^ permalink raw reply
* Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
From: Junio C Hamano @ 2013-01-05 7:54 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <7v1ue0veww.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Adam Spiers <git@adamspiers.org> writes:
>
>> diff --git a/builtin/clean.c b/builtin/clean.c
>> index 0c7b3d0..bd18b88 100644
>> --- a/builtin/clean.c
>> +++ b/builtin/clean.c
>> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>> if (!ignored)
>> setup_standard_excludes(&dir);
>>
>> + add_exclude_list(&dir, EXC_CMDL);
>> for (i = 0; i < exclude_list.nr; i++)
>> add_exclude(exclude_list.items[i].string, "", 0,
>> - &dir.exclude_list[EXC_CMDL]);
>> + &dir.exclude_list_groups[EXC_CMDL].ary[0]);
>
> This looks somewhat ugly for two reasons.
>
> * The abstraction add_exclude() offers to its callers is just to
> let them add one pattern to the list of patterns for the kind
> (here, EXC_CMDL); why should they care about .ary[0] part? Are
> there cases any sane caller (not the implementation of the
> exclude_list_group machinery e.g. add_excludes_from_... function)
> may want to call it with .ary[1]? I do not think of any.
> Shouldn't the public API function add_exclude() take a pointer to
> the list group itself?
>
> * When naming an array of things, we tend to prefer naming it
>
> type thing[count]
>
> so that the second element can be called "thing[2]" and not
> "things[2]". dir.exclude_list_group[EXC_CMDL] reads better.
Also, "ary[]" is a bad name, even as an implementation detail, for
two reasons: it is naming it after its type (being an "array") not
after what it is (if it holds the patterns from the same information
source, e.g. file, togeter, "src" might be a better name), and it
uses rather unusual abbreviation (I haven't seen "array" shortened
to "ary" anywhere else).
>
>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
>> index ef7f99a..c448e06 100644
>> --- a/builtin/ls-files.c
>> +++ b/builtin/ls-files.c
>> @@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,
>> static int option_parse_exclude(const struct option *opt,
>> const char *arg, int unset)
>> {
>> - struct exclude_list *list = opt->value;
>> + struct exclude_list_group *group = opt->value;
>>
>> exc_given = 1;
>> - add_exclude(arg, "", 0, list);
>> + add_exclude(arg, "", 0, &group->ary[0]);
>
> This is another example where the caller would wish to be able to say
>
> add_exclude(arg, "", 0, group);
>
> instead.
^ permalink raw reply
* [PATCH 08/10] t9400: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9400-git-cvsserver-server.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 9502f24..0431386 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -36,6 +36,7 @@ export CVSROOT CVS_SERVER
rm -rf "$CVSWORK" "$SERVERDIR"
test_expect_success 'setup' '
+ git config push.default matching &&
echo >empty &&
git add empty &&
git commit -q -m "First Commit" &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 10/10] push: switch default from "matching" to "simple"
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0.
This finally flips that bit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 6 +++---
builtin/push.c | 31 +++++++------------------------
2 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..770eefe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1750,15 +1750,15 @@ push.default::
since locally stalled branches will attempt a non-fast forward push
if other users updated the branch.
+
- This is currently the default, but Git 2.0 will change the default
- to `simple`.
+ This used to be the default, and stale web sites may still say so,
+ but Git 2.0 has changed the default to `simple`.
* `upstream` - push the current branch to its upstream branch.
With this, `git push` will update the same remote ref as the one which
is merged by `git pull`, making `push` and `pull` symmetrical.
See "branch.<name>.merge" for how to configure the upstream branch.
* `simple` - like `upstream`, but refuses to push if the upstream
branch's name is different from the local one. This is the safest
- option and is well-suited for beginners. It will become the default
+ option and is well-suited for beginners. It has become the default
in Git 2.0.
* `current` - push the current branch to a branch of the same name.
--
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..9f7c252 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -24,7 +24,6 @@ static int progress = -1;
static const char **refspec;
static int refspec_nr;
static int refspec_alloc;
-static int default_matching_used;
static void add_refspec(const char *ref)
{
@@ -148,9 +147,9 @@ static void setup_push_upstream(struct remote *remote, int simple)
}
static char warn_unspecified_push_default_msg[] =
-N_("push.default is unset; its implicit value is changing in\n"
+N_("push.default is unset; its implicit value has changed in\n"
"Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
- "and maintain the current behavior after the default changes, use:\n"
+ "and maintain the traditional behavior, use:\n"
"\n"
" git config --global push.default matching\n"
"\n"
@@ -175,14 +174,14 @@ static void setup_default_push_refspecs(struct remote *remote)
{
switch (push_default) {
default:
- case PUSH_DEFAULT_UNSPECIFIED:
- default_matching_used = 1;
- warn_unspecified_push_default_configuration();
- /* fallthru */
case PUSH_DEFAULT_MATCHING:
add_refspec(":");
break;
+ case PUSH_DEFAULT_UNSPECIFIED:
+ warn_unspecified_push_default_configuration();
+ /* fallthru */
+
case PUSH_DEFAULT_SIMPLE:
setup_push_upstream(remote, 1);
break;
@@ -208,12 +207,6 @@ static const char message_advice_pull_before_push[] =
"before pushing again.\n"
"See the 'Note about fast-forwards' in 'git push --help' for details.");
-static const char message_advice_use_upstream[] =
- N_("Updates were rejected because a pushed branch tip is behind its remote\n"
- "counterpart. If you did not intend to push that branch, you may want to\n"
- "specify branches to push or set the 'push.default' configuration variable\n"
- "to 'simple', 'current' or 'upstream' to push only the current branch.");
-
static const char message_advice_checkout_pull_push[] =
N_("Updates were rejected because a pushed branch tip is behind its remote\n"
"counterpart. Check out this branch and merge the remote changes\n"
@@ -227,13 +220,6 @@ static void advise_pull_before_push(void)
advise(_(message_advice_pull_before_push));
}
-static void advise_use_upstream(void)
-{
- if (!advice_push_non_ff_default || !advice_push_nonfastforward)
- return;
- advise(_(message_advice_use_upstream));
-}
-
static void advise_checkout_pull_push(void)
{
if (!advice_push_non_ff_matching || !advice_push_nonfastforward)
@@ -272,10 +258,7 @@ static int push_with_options(struct transport *transport, int flags)
advise_pull_before_push();
break;
case NON_FF_OTHER:
- if (default_matching_used)
- advise_use_upstream();
- else
- advise_checkout_pull_push();
+ advise_checkout_pull_push();
break;
}
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 09/10] t9401: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9401-git-cvsserver-crlf.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 1c5bc84..8c3db76 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -84,6 +84,7 @@ export CVSROOT CVS_SERVER
rm -rf "$CVSWORK" "$SERVERDIR"
test_expect_success 'setup' '
+ git config push.default matching &&
echo "Simple text file" >textfile.c &&
echo "File with embedded NUL: Q <- there" | q_to_nul > binfile.bin &&
mkdir subdir &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 07/10] t7406: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7406-submodule-update.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index feaec6c..c675ce6 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -565,14 +565,14 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur
git log > ../../../expected
) &&
git commit -m "added subsubmodule" &&
- git push
+ git push origin :
) &&
(cd .git/modules/deeper/submodule/modules/subsubmodule &&
git log > ../../../../../actual
) &&
git add deeper/submodule &&
git commit -m "update submodule" &&
- git push &&
+ git push origin : &&
test_cmp actual expected
)
'
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 06/10] t5531: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5531-deep-submodule-push.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 1947c28..8c16e04 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -16,6 +16,7 @@ test_expect_success setup '
(
cd gar/bage &&
git init &&
+ git config push.default matching &&
>junk &&
git add junk &&
git commit -m "Initial junk"
--
1.8.1.299.gc73b41f
^ permalink raw reply related
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