* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Greg KH @ 2013-02-04 0:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rene Scharfe, git, Konstantin Ryabitsev
In-Reply-To: <7vr4l1gqv8.fsf@alter.siamese.dyndns.org>
On Thu, Jan 31, 2013 at 10:16:27AM -0800, Junio C Hamano wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
>
> > On Thu, Jan 31, 2013 at 09:32:12AM -0800, Junio C Hamano wrote:
> >
> >> How about fixing kup to teach the "let's cheat and let the other end
> >> run 'git archive', if the resulting archive and GPG signature
> >> locally created does match, we do not have to transfer the tarball
> >> itself" trick a fall-back mode that says "but if the signature does
> >> not match, then transfer the bulk used to create the signature to
> >> the remote anyway". This fallback can and should of course be
> >> useful for the compressed patch transfer.
> >
> > Ugh, uploading a 431Mb file, over a flaky wireless connection (I end up
> > doing lots of kernel releases while traveling), would be a horrible
> > change. I'd rather just keep using the same older version of git that
> > kernel.org is running instead.
>
> Then how about fixing kup to try both versions of Git? There will
> be people who run different versions of Git anyway, and kup should
> not be preventing Git from helping people on other platforms, or
> improving its output in general.
I think the combinations of different versions of git that would have to
be installed on kernel.org to handle stuff like this as things change
over time, wouldn't be worth it.
The number of people this affects right now is only one (me), given that
the offending file is not in Linus's tree right now, so he doesn't have
issues with uploading new releases.
So I'll just work to ensure I have the same version of git in place if I
ever run into this problem again. Or just break down and do
full-compressed tarballs instead, if I'm in a place where I have a good
network connection.
thanks,
greg k-h
^ permalink raw reply
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Greg KH @ 2013-02-04 0:48 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Junio C Hamano, Rene Scharfe, git
In-Reply-To: <510AAF4F.6060201@kernel.org>
On Thu, Jan 31, 2013 at 12:52:15PM -0500, Konstantin Ryabitsev wrote:
> On 31/01/13 12:41 PM, Greg KH wrote:
> > Ugh, uploading a 431Mb file, over a flaky wireless connection (I end up
> > doing lots of kernel releases while traveling), would be a horrible
> > change. I'd rather just keep using the same older version of git that
> > kernel.org is running instead.
>
> Well, we do accept compressed archives, so you would be uploading about
> 80MB instead of 431MB, but that would still be a problem for anyone
> releasing large tarballs over unreliable connections. I know you
> routinely do 2-3 releases at once, so that would still mean uploading
> 120-180MB.
That would mean I can't do kernel releases while on ferry rides, which
is probably a good thing in the end :)
> I don't have immediate statistics on how many people release using "kup
> --tar", but I know that at least you and Linus rely on that exclusively.
What causes you to upgrade the version of git on the server? Are you
relying on packages for a distro, or is this "hand installed" by
yourself? As long as I stay in lock-step with your updates, all should
be fine.
Oh, maybe we can report back to the user, the version of git that is
being used on the server, if the checksums don't match, so that I know
to at least see if my version is different from yours?
thanks,
greg k-h
^ permalink raw reply
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Greg KH @ 2013-02-04 0:42 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git, Konstantin Ryabitsev
In-Reply-To: <510AB910.5050504@lsrfire.ath.cx>
On Thu, Jan 31, 2013 at 07:33:52PM +0100, René Scharfe wrote:
> Am 31.01.2013 18:28, schrieb Greg KH:
> > I tracked this down to commit 22f0dcd9634a818a0c83f23ea1a48f2d620c0546
> > (archive-tar: split long paths more carefully). The diff of a hex dump
> > of the tar archives shows the following difference:
> >
> > --- old_git_archive 2013-01-31 17:31:24.466343388 +0100
> > +++ new_git_archive 2013-01-31 17:32:21.509674417 +0100
> > @@ -19239998,8 +19239998,8 @@
> > 125943d0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125943e0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125943f0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > -12594400:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > -12594410:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > +12594400:7765 7374 6272 6964 6765 2d6f 6d61 7033 westbridge-omap3
> > +12594410:2d70 6e61 6e64 2d68 616c 2f00 0000 0000 -pnand-hal/.....
> > 12594420:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 12594430:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 12594440:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > @@ -19240025,8 +19240025,8 @@
> > 12594580:2f61 7374 6f72 6961 2f61 7263 682f 6172 /astoria/arch/ar
> > 12594590:6d2f 706c 6174 2d6f 6d61 702f 696e 636c m/plat-omap/incl
> > 125945a0:7564 652f 6d61 6368 2f77 6573 7462 7269 ude/mach/westbri
> > -125945b0:6467 652f 7765 7374 6272 6964 6765 2d6f dge/westbridge-o
> > -125945c0:6d61 7033 2d70 6e61 6e64 2d68 616c 0000 map3-pnand-hal..
> > +125945b0:6467 6500 0000 0000 0000 0000 0000 0000 dge.............
> > +125945c0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945d0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945e0:0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 125945f0:0000 0000 0000 0000 0000 0000 0000 0000 ................
>
> This is the only directory in the repository whose path is long enough to
> make a difference with the patch, 105 characters in total:
>
> drivers/staging/westbridge/astoria/arch/arm/plat-omap/include/mach/westbridge/westbridge-omap3-pnand-hal/
>
> Five characters less and you wouldn't notice a thing. It contains
> "westbridge" thrice, so I think it's cheating just to reach that
> length, though. ;-)
Yeah, that file path was bad, which is just one reason why it was
deleted from the kernel in newer versions :)
> > Interestingly, the output of uncompressing the tar archives is
> > identical, so the data is correct, but the binary isn't.
>
> The path is split differently between two header fields, that's all.
Ok, thanks for the explaination, I didn't realize that.
> > Now keeping binary compatibility of tar archive files isn't really a big
> > deal, but, the commit to git that causes this seems a bit odd, is it
> > really needed? Or can we just fix the version of tar with NetBSD
> > instead? :)
>
> Apart from Junio's suggestion, I can't think of a practical solution.
>
> You could downgrade your git to a version before the fix. A downside is
> that you won't be able to extract the archive on NetBSD without getting
> an error message (but the contents would be intact, except perhaps for
> permission bits of the directory above).
>
> You could upgrade the kernel.org version of git, but that might cause the
> same problem for other maintainers with long directory paths who in their
> repositories who still use git without the fix.
>
> You could make the path shorter. Won't help at all with the release you
> just did, of course.
What I ended up doing was just to revert your patch, generating a tar
archive that matches what the version on kernel.org.
And originally I now recall that this was something we were worried
about, but we put off dealing with it until it caused problems :)
> I don't know if other tar implementations freak out when they see an
> empty name field. NetBSD's tar might seem a bit too strict here, but
> overall I think it's right in complaining.
Ok, thanks, I now agree.
> What makes the commit odd, by the way?
Sorry, I was originally thinking that you were working around a bug in
the NetBSD version of tar, not making it "more correct", which is
arguably the right thing to do here.
So I'll work with Konstantine to ensure we both are using the same
version of git in the future, it's our kernel.org infrastructure issue
here, not a git one, sorry for the noise.
greg k-h
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jonathan Nieder @ 2013-02-04 3:40 UTC (permalink / raw)
To: Jongman Heo; +Cc: Jeff King, Junio C Hamano, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <19086873.858651359944284535.JavaMail.weblogic@epml01>
Jongman Heo wrote:
> Unfortunately, the patch didn't help to me.
Thanks for testing. Did you apply the patch to the older version of
git that generates builtin/.depend/fetch.o.d or the newer version that
consumes it?
Curious,
Jonathan
^ permalink raw reply
* Re: "git archve --format=tar" output changed from 1.8.1 to 1.8.2.1
From: Junio C Hamano @ 2013-02-04 5:05 UTC (permalink / raw)
To: Greg KH; +Cc: Rene Scharfe, git, Konstantin Ryabitsev
In-Reply-To: <20130204004512.GB6243@kroah.com>
Greg KH <gregkh@linuxfoundation.org> writes:
>> Then how about fixing kup to try both versions of Git? There will
>> be people who run different versions of Git anyway, and kup should
>> not be preventing Git from helping people on other platforms, or
>> improving its output in general.
>
> I think the combinations of different versions of git that would have to
> be installed on kernel.org to handle stuff like this as things change
> over time, wouldn't be worth it.
You make it sound as if you have to pick the right one among 47
different versions, but I think over the lifetime of Git there was
only one time that output from "diff" would have affected the kup's
trick to avoid the transmission of patch text, and another that
output from "tar-tree" (aka "archive --format=tar") would have
afffected the transmission of tarballs. I do think it is feasible
if "kup" wanted to.
> The number of people this affects right now is only one (me), given that
> the offending file is not in Linus's tree right now, so he doesn't have
> issues with uploading new releases.
As a tree grows larger over time, it may be just a matter of time
for somebody else to be hit by another deep path, though.
^ permalink raw reply
* Re: [PATCH v4 1/2] for-each-repo: new command used for multi-repo operations
From: Junio C Hamano @ 2013-02-04 6:41 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Lars Hjemli, Jonathan Nieder, git, Heiko Voigt
In-Reply-To: <5106ECB6.9010801@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Am 28.01.2013 21:34, schrieb Junio C Hamano:
> ...
>> I was imagining that "foreach --untracked" could go something like this:
>>
>> * If you are inside an existing git repository, read its index to
>> learn the gitlinks in the directory and its subdirectories.
>>
>> * Start from the current directory and recursively apply the
>> procedure in this step:
>>
>> * Scan the directory and iterate over the ones that has ".git" in
>> it:
>>
>> * If it is a gitlinked one, show it, but do not descend into it
>> unless --recursive is given (e.g. you start from /home/jens,
>> find /home/jens/proj/ directory that has /home/jens/proj/.git
>> in it. /home/jens/.git/index knows that it is a submodule of
>> the top-level superproject. "proj" is handled, and it is up
>> to the --recursive option if its submodules are handled).
>>
>> * If it is _not_ a gitlinked one, show it and descend into it
>> (e.g. /home/jens/ is not a repository or /home/jens/proj is
>> not a tracked submodule) to apply this procedure recursively.
>>
>> Of course, without --untracked, we have no need to iterate over the
>> readdir() return values; instead we just scan the index of the
>> top-level superproject.
>
> Thanks for explaining, that makes tons of sense.
There is a small thinko above, though, and I'd like to correct it
before anybody takes the above too seriously as _the_ outline of the
design and implements it to the letter.
The --recursive option should govern both a tracked submodule and an
untracked one. When asking to list both existing submodules and
directories that could become submodules, you should be able to say
$ git submodule foreach --untracked
to list the direct submodules and the directories with .git in them
that are not yet submodules of the top-level superproject, but the
latter is limited to those with no parent directories with .git in
them (other than the top-level of the working tree of the
superproject). With
$ git submodule foreach --untracked --recursive
you would see submodules and their submodules recursively, and also
directories with .git in them (i.e. candidates to become direct
submodules of the superproject) and the directories with .git in
them inside such submodule candidates (i.e. candidates to become
direct submodules of the directories that could become direct
submodules of the superproject) recursively.
If we set things up this way:
mkdir -p a/b c/d &&
for d in . a a/b c c/d
do
git init $d &&
( cd $d && git commit --allow-empty -m initial )
done &&
git add a &&
( cd a && git add b )
The expected results for various combinations are:
* "git submodule foreach" would visit 'a' and nothing else;
* "git submodule foreach --recursive" would visit 'a' and 'a/b';
* "git submodule foreach --untracked" would visit 'a' and 'c'; and
* "git submodule foreach --untracked --recursive" would visit all four.
^ permalink raw reply
* Re: Re: Segmentation fault with latest git (070c57df)
From: Jongman Heo @ 2013-02-04 6:58 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, Junio C Hamano, Thomas Rast, git, Antoine Pelisse
Jonathan Nieder wrote:
> Jongman Heo wrote:
>
>> Unfortunately, the patch didn't help to me.
>
>Thanks for testing. Did you apply the patch to the older version of
>git that generates builtin/.depend/fetch.o.d or the newer version that
>consumes it?
>
>Curious,
>Jonathan
Hi, Jonathan,
I applied the patch to newer version.
This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~.
Thanks~!
Best regards,
Jongman Heo
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Junio C Hamano @ 2013-02-04 7:13 UTC (permalink / raw)
To: jongman.heo; +Cc: Jonathan Nieder, Jeff King, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <12070540.431901359961105650.JavaMail.weblogic@epml10>
Jongman Heo <jongman.heo@samsung.com> writes:
> Jonathan Nieder wrote:
>> Jongman Heo wrote:
>>
>>> Unfortunately, the patch didn't help to me.
>>
>>Thanks for testing. Did you apply the patch to the older version of
>>git that generates builtin/.depend/fetch.o.d or the newer version that
>>consumes it?
>>
>>Curious,
>>Jonathan
>
> Hi, Jonathan,
>
> I applied the patch to newer version.
>
> This time, I tried to apply the patch to older version of Makefile, and now the issue is fixed~.
> Thanks~!
Yeah, that result is understandable, as .depend/*.o.d files will not
be rebuilt when the rules to build them changes in the Makefile.
Applying the patch to the Makefile in the pristine old tree, run the
build (which will generate .depend/*.o.d files with the corrected
rules), then checking out the new tree and running the build again
without "make clean", with or with the patch applied, would validate
that the patch fixes the issue for old ccache.
Thanks Jonathan for diagnosing, fixing, and thanks Jongman for
testing.
^ permalink raw reply
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Duy Nguyen @ 2013-02-04 7:14 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jonathan Niedier
In-Reply-To: <vpqpq0hnlb1.fsf@grenoble-inp.fr>
On Mon, Feb 4, 2013 at 4:23 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> --- a/t/t6030-bisect-porcelain.sh
>> +++ b/t/t6030-bisect-porcelain.sh
>> @@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
>> cp .git/BISECT_START saved &&
>> test_must_fail git bisect start $HASH4 foo -- &&
>> git branch > branch.output &&
>> - test_i18ngrep "* (no branch)" branch.output > /dev/null &&
>> + test_i18ngrep "* (bisecting other)" branch.output > /dev/null &&
>
> I'd have spelled it (no branch, bisecting other) to make it clear that
> we're on detached HEAD, and avoid confusing old-timers. But maybe your
> version is enough, I'm not sure.
If we want to make it clear, I think the standard "* (no branch)" should become
* HEAD (detached)
or non-detached case:
* HEAD -> foo
Then we could present rebase/bisect information as
* HEAD (detached, bisecting)
* HEAD (detached, rebasing)
* foo (rebasing)
I don't want to make this line too long because it would break (well,
waste space in) column layout. So if we do this, no branch name added
for rebase/bisect.
--
Duy
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Junio C Hamano @ 2013-02-04 7:17 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, git
In-Reply-To: <20130201185827.GA22919@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I was specifically thinking of this (on top of your patch):
>
> diff --git a/remote-curl.c b/remote-curl.c
> index e6f3b63..63680a8 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char *service)
> last->buf_alloc = strbuf_detach(&buffer, &last->len);
> last->buf = last->buf_alloc;
>
> - if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (maybe_smart && !strbuf_cmp(&exp, &type)) {
> /*
> * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - strbuf_addf(&exp, "application/x-%s-advertisement", service);
> - if (strbuf_cmp(&exp, &type))
> - die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
>
> To just follow the dumb path if we don't get the content-type we expect.
> We may want to keep the '#' format check in addition (packet_get_line
> will check it and die, anyway, but we may want to drop back to
> considering it dumb, just to protect against a badly configured dumb
> server which uses our mime type, but I do not think it likely).
Yeah, but it doesn't cost anything to check, so let's do so.
Does this look good to both of you (relative to Shawn's patch)?
remote-curl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
last->buf_alloc = strbuf_detach(&buffer, &last->len);
last->buf = last->buf_alloc;
- if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+ strbuf_addf(&exp, "application/x-%s-advertisement", service);
+ if (maybe_smart &&
+ (5 <= last->len && last->buf[4] == '#') &&
+ !strbuf_cmp(&exp, &type)) {
/*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
- strbuf_addf(&exp, "application/x-%s-advertisement", service);
- if (strbuf_cmp(&exp, &type))
- die("invalid content-type %s", type.buf);
if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
die("%s has invalid packet header", refs_url);
if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
^ permalink raw reply related
* Re: [PATCH v2] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Duy Nguyen @ 2013-02-04 7:17 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jonathan Niedier
In-Reply-To: <CACsJy8CGqiahw3y42KRt61gChtfOucFHqZqn_uvLrj7j7KrQbg@mail.gmail.com>
On Mon, Feb 4, 2013 at 2:14 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> * foo (rebasing)
Well, this one does not make sense (or causes more confusion).
--
Duy
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-04 8:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vehgw5z7n.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 11:13:00PM -0800, Junio C Hamano wrote:
> Yeah, that result is understandable, as .depend/*.o.d files will not
> be rebuilt when the rules to build them changes in the Makefile.
> Applying the patch to the Makefile in the pristine old tree, run the
> build (which will generate .depend/*.o.d files with the corrected
> rules), then checking out the new tree and running the build again
> without "make clean", with or with the patch applied, would validate
> that the patch fixes the issue for old ccache.
>
> Thanks Jonathan for diagnosing, fixing, and thanks Jongman for
> testing.
Do we want to do anything with the other dependency hole I found here:
http://article.gmane.org/gmane.comp.version-control.git/215211
It's definitely a potential problem, but I don't think we have any
reports of it happening in practice, so it might not be worth worrying
about. Doing a clean version of the fix here:
http://article.gmane.org/gmane.comp.version-control.git/215212
would probably involve reorganizing our .depend directory structure,
unless somebody can cook up some clever use of make's patsubst.
-Peff
^ permalink raw reply
* Re: [PATCH] Verify Content-Type from smart HTTP servers
From: Jeff King @ 2013-02-04 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Pearce, git
In-Reply-To: <7va9rk5z02.fsf@alter.siamese.dyndns.org>
On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:
> Does this look good to both of you (relative to Shawn's patch)?
>
> remote-curl.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index e6f3b63..933c69a 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
> last->buf_alloc = strbuf_detach(&buffer, &last->len);
> last->buf = last->buf_alloc;
>
> - if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
> + strbuf_addf(&exp, "application/x-%s-advertisement", service);
> + if (maybe_smart &&
> + (5 <= last->len && last->buf[4] == '#') &&
> + !strbuf_cmp(&exp, &type)) {
> /*
> * smart HTTP response; validate that the service
> * pkt-line matches our request.
> */
> - strbuf_addf(&exp, "application/x-%s-advertisement", service);
> - if (strbuf_cmp(&exp, &type))
> - die("invalid content-type %s", type.buf);
> if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
> die("%s has invalid packet header", refs_url);
> if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
Yeah, I think that's fine. Thanks.
-Peff
^ permalink raw reply
* Re: Getting started contributing.
From: adamfraser @ 2013-02-04 8:42 UTC (permalink / raw)
To: git
In-Reply-To: <7v1ucx9o4m.fsf@alter.siamese.dyndns.org>
Thanks for all of the replies. I'll have a look into the suggestions and try
to find somewhere I can help out. :)
--
View this message in context: http://git.661346.n2.nabble.com/Getting-started-contributing-tp7576834p7576901.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Junio C Hamano @ 2013-02-04 9:16 UTC (permalink / raw)
To: Jeff King; +Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <20130204083701.GA30835@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Do we want to do anything with the other dependency hole I found here:
>
> http://article.gmane.org/gmane.comp.version-control.git/215211
>
> It's definitely a potential problem, but I don't think we have any
> reports of it happening in practice, so it might not be worth worrying
> about. Doing a clean version of the fix here:
>
> http://article.gmane.org/gmane.comp.version-control.git/215212
>
> would probably involve reorganizing our .depend directory structure,
> unless somebody can cook up some clever use of make's patsubst.
As I understand how the current set-up works:
* Initially, we do not have foo.o but foo.c. We automatically
build foo.o because it depends on foo.c via the "%.o : %.c" rule,
and as a side effect, we also build .depend/foo.o.d file;
* Then, if any real dependency used to build the existing foo.o
that is recorded in .depend/foo.o.d file changes, foo.o gets
rebuilt, which would update .depend/foo.o.d again for the next
invocation.
The case where you lose .depend/foo.o.d is a special case of getting
a wrong information in .depend/foo.o.d, which may happen by using a
broken compiler during the initial build, or going over quota and
getting .depend/foo.o.d truncated, or by other breakages. The user
may have done "rm -rf .depend" to lose it, or the user may have done
something like this to munge it:
find -name '.git' -type d -prune -o -print0 |
xargs -0 sed -i -e 's/foo/bar/g'
forgetting that just like .git, .depend is precious and should not
be touched.
I think this really boils down to where we draw the "this is good
enough" line. I am not sure if losing the file as in $gmane/215211
is common enough to be special cased to buy us much, while leaving
other ".depend/foo.o.d was updated to contain a wrong info" cases
still broken.
And of course the case where .depend/foo.o.d is munged by mistake
cannot be solved without recompiling everything all the time, so...
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Junio C Hamano @ 2013-02-04 9:29 UTC (permalink / raw)
To: Jeff King; +Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vsj5c4exz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> As I understand how the current set-up works:
>
> * Initially, we do not have foo.o but foo.c. We automatically
> build foo.o because it depends on foo.c via the "%.o : %.c" rule,
> and as a side effect, we also build .depend/foo.o.d file;
>
> * Then, if any real dependency used to build the existing foo.o
> that is recorded in .depend/foo.o.d file changes, foo.o gets
> rebuilt, which would update .depend/foo.o.d again for the next
> invocation.
This is unrelated to the case you mentioned, but I wonder what
happens if you did this:
* You are on branch 'next', where foo.c includes (perhaps
indirectly) frotz.h. Compile and you get foo.o and also the
dependency recorded for it, "foo.o: foo.c frotz.h", in the
.depend/foo.o.d file.
* You check out branch 'master', where foo.c does not include
frotz.h. Indeed, the include file does not even exist on the
branch.
Do we get confused, because Makefile includes the depend file from
the previous build, finds that you need foo.c and frotz.h up to date
in order to get foo.o, but there is no rule to generate frotz.h?
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-04 9:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vsj5c4exz.fsf@alter.siamese.dyndns.org>
On Mon, Feb 04, 2013 at 01:16:08AM -0800, Junio C Hamano wrote:
> I think this really boils down to where we draw the "this is good
> enough" line. I am not sure if losing the file as in $gmane/215211
> is common enough to be special cased to buy us much, while leaving
> other ".depend/foo.o.d was updated to contain a wrong info" cases
> still broken.
Hmm. Yeah, I was thinking it might be more common than ordinary munging
due to something like an interrupted "git clean -x". But given that:
1. As far as I can tell, it is not a situation that can happen through
regular use of checkout/make/etc, and...
2. We have zero reports of it happening in practice (I only discovered
it while explicitly trying to break the Makefile), and...
3. It is just one of many possible breakages, all of which can be
fixed by "git clean -dx" if you suspect issues...
let's just leave it. Thanks for a sanity check.
-Peff
^ permalink raw reply
* Re: Segmentation fault with latest git (070c57df)
From: Jeff King @ 2013-02-04 9:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: jongman.heo, Jonathan Nieder, Thomas Rast, git, Antoine Pelisse
In-Reply-To: <7vobg04ebe.fsf@alter.siamese.dyndns.org>
On Mon, Feb 04, 2013 at 01:29:41AM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > As I understand how the current set-up works:
> >
> > * Initially, we do not have foo.o but foo.c. We automatically
> > build foo.o because it depends on foo.c via the "%.o : %.c" rule,
> > and as a side effect, we also build .depend/foo.o.d file;
> >
> > * Then, if any real dependency used to build the existing foo.o
> > that is recorded in .depend/foo.o.d file changes, foo.o gets
> > rebuilt, which would update .depend/foo.o.d again for the next
> > invocation.
>
> This is unrelated to the case you mentioned, but I wonder what
> happens if you did this:
>
> * You are on branch 'next', where foo.c includes (perhaps
> indirectly) frotz.h. Compile and you get foo.o and also the
> dependency recorded for it, "foo.o: foo.c frotz.h", in the
> .depend/foo.o.d file.
>
> * You check out branch 'master', where foo.c does not include
> frotz.h. Indeed, the include file does not even exist on the
> branch.
>
> Do we get confused, because Makefile includes the depend file from
> the previous build, finds that you need foo.c and frotz.h up to date
> in order to get foo.o, but there is no rule to generate frotz.h?
No, because the .d files look like this:
foo.o: frotz.h
frotz.h:
So make sees that it can build frotz.h, which of course does nothing.
But that's OK, because foo.c doesn't actually include it anymore, and
when we recompile it (as we must, since it is different between the two
branches), we will rewrite the .d file without frotz.h.
-Peff
^ permalink raw reply
* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Michael J Gruber @ 2013-02-04 11:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmwvl6lj9.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 04.02.2013 00:10:
> I think a natural way to ask reviewing the recent merges while
> showing tricky ones would be to say:
>
> $ git log --first-parent --cc master..pu
>
> But this does not to show what I expect to see, which is an output
> of:
>
> $ git log --first-parent --cc -p master..pu
>
I'm not Junio, but I guess he would respond that it's a matter of
expectations: "--cc" is a diff option, and just like any other diff
option, it has an effect on "log" only if "log" has been told to show a
diff.
Oh wait, you *are* Junio ;)
> This is only a minor irritation, but I think it might make sense to
> make it notice the --cc in the former and turn -p on automatically.
I think we have a clear distiction between rev-list/log options and diff
options. "log" comes without a diff, "-p" turns on diffs.
"log" passes diff-options to "diff". If the user gives a diff-option to
"log" we can conclude that he meant to request a diff and turn them on
automatically (as if "-p" was there also).
But I'm wondering whether that has adverse effects on scripts/aliases.
For example, I could have a special alias
newpu = log first-parent --cc next..pu
which allows me to use "git newpu" as well as "git newpu -p" to get
those new commits with or without diff in my preferred format, but not
any more after the change you suggest. (I could use a second alias, of
course; and yes, I'm (ab)using current option parser features.)
> The same for
>
> $ git log --cc next~3..next
>
> which may make sense to turn into "git log -p --cc next~3..next".
>
> When deciding if the above makes sense, there are a few things to
> know to be true as prerequisites for the discussion:
>
> * Neither of these
>
> $ git log --first-parent -p master..pu
> $ git log -p master..pu
>
> shows any patches, and it is not a bug. No patches are shown for
> merges unless -m is given, and when -m is given, we give pairwise
> 2-way diffs for the number of parents.
But diffs are on here ("-p"), it's just that the default diff option for
merges is to not display them. Well, I admit there's two different ways
of thinking here:
A) "git log -p" turns on diffs for all commits, and the default diff
options is the (non-existing) "--no-show" diff-option for merges.
B) "git log -p" applies "-p" to all commits except merge commits.
I'm strongly in the A camp, because I think that this gives a clearer
interface. A really describes the user facing side, whereas B is closer
to the implementation.
> * We recently tweaked this:
>
> $ git log --first-parent -m -p master..pu
>
> to omit diffs with second and later parents, as that is what the
> user wishes with --first-parent.
That made --first-parent into a dual-purpose option, i.e. it modifies
the rev-listing to one parent as well as the diff creation. It does not
turn on diffs by itself.
> * The "--cc" option, when comparing two trees (i.e. showing a
> non-merge commit), is designed to show a normal patch. In other
> words, you can view "--cc" as a modifier when you request a patch
> output format with "-p". For "git show", "--cc -p" is turned on
> by default, and giving "-m" explicity (i.e. "git show -m") you
> can turn it off and have it do "-m -p" instead.
>
Yes, I really think of --cc is a diff-option, and that this point of
view gives the clearest ui.
We could argue that any diff-option could switch on diffs (imply -p),
but that change seems to be quite radical. "show" having "-p" as a
default is quite natural, but that is different.
Having only "--cc" imply "-p" would be too much special case auto-magic
for my taste. We have too many of these already.
I really think we should leave it as is.
Michael
^ permalink raw reply
* Re: Feature request: Allow extracting revisions into directories
From: Michael J Gruber @ 2013-02-04 11:18 UTC (permalink / raw)
To: Robert Clausecker; +Cc: git
In-Reply-To: <1359901085.24730.11.camel@t520>
Robert Clausecker venit, vidit, dixit 03.02.2013 15:18:
> Hello!
>
> git currently has the archive command that allows to save an arbitrary
> revision into a tar or zip file. Sometimes it is useful to not save this
> revision into an archive but to directly put all files into an arbitrary
> directory. Currently this seems to be not possible to archive directly;
> the only way I found to do it is to run git archive and then directly
> unpack the archive into a directory.
>
> git --git-dir REPO archive REVISION | tar x
>
> It would be nice to have a command or simply a switch to git archive
> that allows the user to put the files of REVISION into a directory
> instead of making an archive.
>
> Thank you very much for your help. Yours,
>
> Robert Clausecker
Sitaram has said much about the Unix philosophy already, and Konstantin
gave a variant of checkout. Just two more cents:
How would you copy a directory tree? I presume you wouldn't use "tar c .
| tar -xC gothere", but what would be your worklflow?
Depending on what you actually want to achieve, "git clone -b branch"
and removing the superfluous .git-dir might be a viable option. (Beware
the hardlinks, though.)
Michael
^ permalink raw reply
* Re: Feature request: Allow extracting revisions into directories
From: Robert Clausecker @ 2013-02-04 12:14 UTC (permalink / raw)
To: Michael J Gruber, git
In-Reply-To: <510F9907.7010107@drmicha.warpmail.net>
[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]
Am Montag, den 04.02.2013, 12:18 +0100 schrieb Michael J Gruber:
> Sitaram has said much about the Unix philosophy already, and Konstantin
> gave a variant of checkout. Just two more cents:
>
> How would you copy a directory tree? I presume you wouldn't use "tar c .
> | tar -xC gothere", but what would be your worklflow?
>
> Depending on what you actually want to achieve, "git clone -b branch"
> and removing the superfluous .git-dir might be a viable option. (Beware
> the hardlinks, though.)
>
> Michael
The specific workflow I am planning is this:
I have a server that hosts a bare git repository. This git repository
contains a branch production. Whenever somebody pushes to production a
hook automatically puts a copy of the current production branch
into /var/www/foo. I could of course use pull for that but it just does
not feels right. Why should I have a repository twice on the server?
Adding an option to put the tree of an arbitrary revision into a
directory is something that improves usability as it is an operation
semantically different from tar. Saying that you can already get this
with git archive and ad-hoc unpacking is as saying: You don't need cp.
Just tar the file and untar it somewhere else.
Of course that is a possibility but it does not not feel right and is
not intuitive. Adding this feature won't cause feature creep but would
rather add an operation that makes sense in some scenarios and reduces
the dependencies on other commands that might not be available on other
platforms (If you care about that).
Also, this functionality is in full accordance with the Unix principle
as it is a basic operation ("put tree into files") and not something
super special. Also, it always feels like a hack if you do this ad-hoc
unpacking. Like "git can't do it the simple way".
Yours, Robert Clausecker
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: Feature request: Allow extracting revisions into directories
From: Tomas Carnecky @ 2013-02-04 12:47 UTC (permalink / raw)
To: Robert Clausecker, Michael J Gruber, git
In-Reply-To: <1359980045.24730.32.camel@t520>
On Mon, 04 Feb 2013 13:14:05 +0100, Robert Clausecker <fuzxxl@gmail.com> wrote:
> Of course that is a possibility but it does not not feel right and is
> not intuitive. Adding this feature won't cause feature creep but would
> rather add an operation that makes sense in some scenarios and reduces
> the dependencies on other commands that might not be available on other
> platforms (If you care about that).
I'd really like to see your reply to Sitaram's email regarding the many
options that tar has. Sure, just teaching git-archive the equivalent of `|tar
-x' probably isn't feature creep. But why stop there and not add some of the
other options as well? After all, some might be equally useful in a different
situation. So where do you stop? When you've completely merged tar into git?
> Also, this functionality is in full accordance with the Unix principle
> as it is a basic operation ("put tree into files") and not something
> super special.
That's what `git checkout` is for. And I would even argue that it's the better
choice in your situation because it would delete files from /var/www/foo which
you have deleted in your repo. git archive|tar wouldn't do that.
^ permalink raw reply
* [PATCH 1/2] git-count-objects.txt: describe each line in -v output
From: Nguyễn Thái Ngọc Duy @ 2013-02-04 12:49 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
The current description requires a bit of guessing (what clause
corresponds to what printed line?) and lacks information, such as
the unit of size and size-pack.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-count-objects.txt | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 23c80ce..e816823 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -20,11 +20,21 @@ OPTIONS
-------
-v::
--verbose::
- In addition to the number of loose objects and disk
- space consumed, it reports the number of in-pack
- objects, number of packs, disk space consumed by those packs,
- and number of objects that can be removed by running
- `git prune-packed`.
+ Report in more detail:
++
+count: the number of loose objects
++
+size: disk space consumed by loose objects, in KiB
++
+in-pack: the number of in-pack objects
++
+size-pack: disk space consumed by the packs, in KiB
++
+prune-packable: the number of loose objects that are also present in
+the packs. These objects could be pruned using `git prune-packed`.
++
+garbage: the number of files in loose object database that are not
+valid loose objects
GIT
---
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Nguyễn Thái Ngọc Duy @ 2013-02-04 12:49 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1359982145-10792-1-git-send-email-pclouds@gmail.com>
While it's unusual to have strange files in loose object database,
.git/objects/pack/tmp_* is normal after a broken fetch and they
can eat up a lot of disk space if the user does not pay attention.
Report them.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
The hook in prepare_packed_git_one is ugly, but I don't want to
duplicate the search file logic there in count-objects. Maybe I'm
wrong.
Interestingly it reports .commits file in my repo too. A nice
reminder to myself to remind Jeff about count-objects improvements
for his commit-cache work :)
Way may also need a more friendly format than this one, which I
assume is plumbing. Something that average git user can understand
without looking up the document. If "git stats" is too much for this
purpose, perhaps "git gc --stats"?
Documentation/git-count-objects.txt | 4 ++--
builtin/count-objects.c | 27 ++++++++++++++++++++++++++-
sha1_file.c | 23 ++++++++++++++++++++---
3 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..e8fabcf 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,8 @@
#include "builtin.h"
#include "parse-options.h"
+static unsigned long garbage;
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -65,6 +67,27 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
}
}
+extern void (*report_pack_garbage)(const char *path, int len, const char *name);
+static void real_report_pack_garbage(const char *path, int len, const char *name)
+{
+ if (is_dot_or_dotdot(name))
+ return;
+ if (has_extension(name, ".pack")) {
+ struct strbuf idx_file = STRBUF_INIT;
+ struct stat st;
+
+ strbuf_addf(&idx_file, "%.*s/%.*s.idx", len, path,
+ (int)strlen(name) - 5, name);
+ if (!stat(idx_file.buf, &st) && S_ISREG(st.st_mode)) {
+ strbuf_release(&idx_file);
+ return;
+ }
+ strbuf_release(&idx_file);
+ }
+ error("garbage found: %.*s/%s", len, path, name);
+ garbage++;
+}
+
static char const * const count_objects_usage[] = {
N_("git count-objects [-v]"),
NULL
@@ -76,7 +99,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -87,6 +110,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_pack_garbage = real_report_pack_garbage;
memcpy(path, objdir, len);
if (len && objdir[len-1] != '/')
path[len++] = '/';
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..6045946 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1000,6 +1000,17 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+static void dummy_report_pack_garbage(const char *path,
+ int len,
+ const char *name)
+{
+}
+
+void (*report_pack_garbage)(const char *path,
+ int len,
+ const char *name) =
+ dummy_report_pack_garbage;
+
static void prepare_packed_git_one(char *objdir, int local)
{
/* Ensure that this buffer is large enough so that we can
@@ -1024,11 +1035,15 @@ static void prepare_packed_git_one(char *objdir, int local)
int namelen = strlen(de->d_name);
struct packed_git *p;
- if (!has_extension(de->d_name, ".idx"))
+ if (!has_extension(de->d_name, ".idx")) {
+ report_pack_garbage(path, len - 1, de->d_name);
continue;
+ }
- if (len + namelen + 1 > sizeof(path))
+ if (len + namelen + 1 > sizeof(path)) {
+ report_pack_garbage(path, len - 1, de->d_name);
continue;
+ }
/* Don't reopen a pack we already have. */
strcpy(path + len, de->d_name);
@@ -1042,8 +1057,10 @@ static void prepare_packed_git_one(char *objdir, int local)
* .pack file that we can map.
*/
p = add_packed_git(path, len + namelen, local);
- if (!p)
+ if (!p) {
+ report_pack_garbage(path, len - 1, de->d_name);
continue;
+ }
install_packed_git(p);
}
closedir(dir);
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* Re: Feature request: Allow extracting revisions into directories
From: Andrew Ardill @ 2013-02-04 12:58 UTC (permalink / raw)
To: Robert Clausecker; +Cc: Michael J Gruber, git@vger.kernel.org
In-Reply-To: <1359980045.24730.32.camel@t520>
On 4 February 2013 23:14, Robert Clausecker <fuzxxl@gmail.com> wrote:
> The specific workflow I am planning is this:
>
> I have a server that hosts a bare git repository. This git repository
> contains a branch production. Whenever somebody pushes to production a
> hook automatically puts a copy of the current production branch
> into /var/www/foo. I could of course use pull for that but it just does
> not feels right. Why should I have a repository twice on the server?
Maybe I'm missing something. How does the behaviour you need differ from
> GIT_WORKING_DIR=/var/www/foo git checkout -f <tree-ish>
??
Regards,
Andrew Ardill
^ 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