Git development
 help / color / mirror / Atom feed
* Re: tracking submodules out of main directory.
From: henri GEIST @ 2011-08-05  0:29 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Junio C Hamano, Heiko Voigt, Alexei Sholik, git, Sverre Rabbelier
In-Reply-To: <4E3ADAD0.1060800@web.de>

Le jeudi 04 août 2011 à 19:45 +0200, Jens Lehmann a écrit :
> Am 04.08.2011 00:29, schrieb henri GEIST:
> > Le mercredi 03 août 2011 à 23:30 +0200, Jens Lehmann a écrit :
> >> Am 03.08.2011 21:41, schrieb Junio C Hamano:
> >>> Jens Lehmann <Jens.Lehmann@web.de> writes:
> >> But when you fetch a new version of Gimp into your submodule, it would be
> >> really nice if the superproject could be notified that the Gimp developers
> >> updated to 1.2.4 of Glib and inform you that an update of Glib might be
> >> appropriate. That could avoid having you to dig through compiler errors to
> >> find out that the new foobar() function from Glib 1.2.4 is needed (and if
> >> you need to pull in a bugfix in Glib, you might notice that *a lot* later
> >> when you forget to do that).
> >>
> > 
> > Exact, I am really happy to read this.
> > And better do not bother to have the suproject.
> 
> I don't get this, you can't get rid of the superproject.

Of corse I can.
I have done It the patch I have submit has been done for an do it
successfully.
I am doing it every day.

I have in the same "directory" not repository.
Two repository "gimp" and "libpng".
I cd in the gimp directory.
And do "git add ../libpng" and it is done.
The gimp repository track the "../libpng" repository by its path and
sha1.

I do not need anything else.

> 
> > cd to gimp directory, type git status it can tell you every thing and
> > when your satisfied you just have to type make.
> > At this point the superproject have not any use. 
> 
> "git status" inside the submodule won't tell you anything about the
> dependencies,

Of corse It does if the repository add been add
It will just like any other submodules inside of the repository
Git status is even unable to see the difference.
Essential because there is no difference.

> but a "git status" in the superproject should.

Yes but I have no and want not to have a superproject.

> The submodule shouldn't know where exactly the submodules it depends on lives, that is
> the decision of the superproject, not the submodule.

Why not in my case it is needed has in "gimp.c" I have

#include "../libpng.h"

If the code need to know where is libpng why the repository should not
know it as well ?

> 
> >>>> In addition to that, it can (but mustn't) specify any of the following:
> >>>
> >>> I am guessing you meant "does not have to", instead of mustn't, here...
> >>
> >> Sure, thanks for deciphering that.
> >>
> >>>> a) Of this submodule "foo" I need at least that version because I won't
> >>>>    compile/work with older versions of that. (this can be tightened to
> >>>>    "exactly that version" to give henri the behavior he wants, but that
> >>>>    should be policy, not mandatory)
> >>>
> >>> The "loose collection of projects" approach like that has its uses, and it
> >>> is called "repo". Why re-invent it? The behaviour Henri wants to specify
> >>> the exact version is how git submodules work already, so I do not see
> >>> there is anything to be done here.
> >>
> >> Let me make this clear: this is not about changing how submodules are
> >> committed in a superproject. It is not about having a loose collection of
> >> projects, they stay tied together in a defined state by the superproject.
> >>
> > 
> > Yes but for me, from when I started this this topic, it was all about
> > having a loose collection of project with dependency references between
> > them. And get rid of the superproject.
> > It is my first and only goal.
> 
> But I fail to see how that would improve anything.
> 

It improve my life and the life of my team has developers everyday.
All my team agree on this point. In fact they asked me to implement it.
Because we used to have a superproject and it does not scale well. 


The "gimp" team and the "gqview" team shares the libpng but at there
point of view they are not related they even not need to know that the
other one exist. Then they will never share the same superproject. It
has no sens for them.
Then in your model you will need a superproject by project.
I think we should put all that you put in the superproject in the
project itself. Then there will be only one project by project.

And if the dependency are not part of the superproject, this remove the
burden of each team to redefine all the dependencies recursively in any
dependency they have. has the original "libpng" team have done it in the
"libpng" project and not in there own superprojec witch the can not
share.

> >> Henri wanted it a bit upside down: any submodule could request a certain
> >> version of another submodule somewhere else in the repo. And he wanted to
> >> use gitlinks from one submodule to another for that, which I - hopefully -
> >> convinced him was no good idea.
> >>
> > 
> > You just convince me that submodules are more than I need and to make a
> > lighter independent version of submodules which will never been followed
> > by git commands.
> 
> Submodules are what you need, but it's no use to implement dependencies by
> using gitlinks that point outside of their repositories.
> 

Submodules is not what I need.
What I need is pointers to external repository tracked by git.
Submodule was to my eyes the logical tool to do it.
And I had made a patch to enable the tracking of external repository
just has they do for internal ones.

You convince me that you think prohibiting me to do so is a feature to
your eyes. Because gitlinks and submodules can do more than this and you
do not want those other things to work outside of the repository.

Then I propose to cook a kind of "my_light_gitlinks" which do only that.

> >>>> b) And if you don't know where to get it, use this url
> >>>
> >>> Again that is the job of .gitmodules in the superproject.
> >>
> >> Yes. But this idea is about how the url could get into the .gitmodules of
> >> the superproject in the first place. That can make it easier for the
> >> superproject's developer to import a submodule into his repo and much more
> >> important: it makes it possible to pull in submodule dependencies
> >> automatically e.g. when running "git submodule add --resolve-dependencies
> >> Gimp".
> > 
> > Only if you have a superproject.
> > If not do the same thing from the gimp repository, now it contain all
> > necessary infos to do the job.
> 
> No, it doesn't. Apart from the Gimp project telling you what version it
> wants, you need to have a place to record the version that the user really
> used. And that won't work without a superproject.
> 

the version of "libpng" that the user really use is stored in his
"libpng" repository !
And tracked by every project depending on it.

> >>>> That is all stuff the submodule knows better than the superproject.
> >>>
> >>> Not necessarily. The version A0 of submodule A may depend on submodule B
> >>> and may also know it must have at least version B0 of that submodule, but
> >>> the superproject would know other constraints, e.g. the superproject
> >>> itself also calls into submodule B and wants a newer version B1 of it.
> >>
> >> Right. That's what I tried to explain to Henri, the superproject ties it all
> >> together. But I also like his idea to add a way to communicate information
> >> from the submodule to the superproject, and give the superproject a choice
> >> if it wants to use it.
> >>
> > 
> > yes but the superproject contain no code in your design.
> > Then it will never need anything by itself.
> > It is only a container which you will inform with data already known by
> > the submodules I do not see any value to it.
> 
> But being the container that ties it all together is more than enough value.
> 

It has absolutely no value to have a git repository as a container.
A simple directory will do it very well.

	Henri

^ permalink raw reply

* [tig] [PATCH] Display line numbers in main view
From: Kumar Appaiah @ 2011-08-05  1:06 UTC (permalink / raw)
  To: git; +Cc: Jonas Fonseca

When line numbers are enabled, this enables the display of line
numbers in the main view as well. This is useful for cases where one
would want to quickly navigate to a particular commit by the ordering
displayed in tig, using the `:<num>' approach.
---
 tig.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tig.c b/tig.c
index ed92400..92091ab 100644
--- a/tig.c
+++ b/tig.c
@@ -6194,6 +6194,9 @@ main_draw(struct view *view, struct line *line, unsigned int lineno)
 	if (!commit->author)
 		return FALSE;
 
+	if (opt_line_number && draw_lineno(view, lineno))
+		return TRUE;
+
 	if (draw_date(view, &commit->time))
 		return TRUE;
 
-- 
1.7.2.5

^ permalink raw reply related

* Re: tracking submodules out of main directory.
From: henri GEIST @ 2011-08-05  2:19 UTC (permalink / raw)
  To: Heiko Voigt
  Cc: Jens Lehmann, Junio C Hamano, Alexei Sholik, git,
	Sverre Rabbelier
In-Reply-To: <20110804200529.GA35398@book.hvoigt.net>

Le jeudi 04 août 2011 à 22:05 +0200, Heiko Voigt a écrit :
> Hi,
> 
> On Thu, Aug 04, 2011 at 12:29:22AM +0200, henri GEIST wrote:
> > Le mercredi 03 ao??t 2011 ?? 23:30 +0200, Jens Lehmann a ??crit :
> > > Let me make this clear: this is not about changing how submodules are
> > > committed in a superproject. It is not about having a loose collection of
> > > projects, they stay tied together in a defined state by the superproject.
> > > 
> > 
> > Yes but for me, from when I started this this topic, it was all about
> > having a loose collection of project with dependency references between
> > them. And get rid of the superproject.
> > It is my first and only goal.
> 
> In that case maybe what you want is your own completely independent
> implementation of a tool which manages a collection of submodules.

If I make a tool to managed a collection of submodules, it will became
the superproject I want to get rid of.
I do not want to have any concept of a collection. And that is why I do
not want a superproject.

I have no exhaustive list of project/modules/submodules.
Only some independent projects witch knows if they need some others.

Then I realy need the tool to be embedded in the repository.

> I doubt that tracking of submodules outside of the worktree will ever be
> accepted inside core git. Some dependency scheme which makes use of the
> current submodule implementation could be a feasible way but you can not
> get rid of the superproject.

I can not but have already done it.

> 
> Your approach introduces many problems which you were not able to
> present solutions for.

But it does not break anything.
And generate no problem for me.
There is only when you try to get one step more and try to use even more
advanced things than I do on top of it.
And those problem also apply to normal submodules.

> So if you really want to work on this I suggest
> you try to implement your solution outside of core git first. If you can
> prove that you can solve all the immanent problems we can discuss
> integrating it into git again.
> 

That is what I had proposed to do by making a file .gitdependencies
similar to .gitmodules and create shell script wrappers for :
  - git add
  - git status
  - git reset
  - git rm

the only thing is that with the previous patch I only had to touch "git
add" every thing else was already ready to handle it.

> > > Henri wanted it a bit upside down: any submodule could request a certain
> > > version of another submodule somewhere else in the repo. And he wanted to
> > > use gitlinks from one submodule to another for that, which I - hopefully -
> > > convinced him was no good idea.
> > > 
> > 
> > You just convince me that submodules are more than I need and to make a
> > lighter independent version of submodules which will never been followed
> > by git commands.
> 
> As described above this is probably the best way for you. Maybe you can
> prove that such a tool works but git's submodules implementation can
> currently not really assist you with your approach.

We will see.

	Henri

^ permalink raw reply

* Re: [RFC 4/6] git-check-attr: Normalize paths
From: Michael Haggerty @ 2011-08-05  6:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vliv95cpb.fsf@alter.siamese.dyndns.org>

On 08/04/2011 07:05 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>>> I think passing down "prefix" (i.e. where your $(cwd) was relative to the
>>> root level of the working tree) and the user-supplied "pathspec" (which
>>> typically is relative to that original $(cwd)) is the canonical approach.
>>> The very original git worked only at the root level of the working tree,
>>> with paths specified relative to the root level of the tree, so many code
>>> do:
>>>
>>> 	- find out the root of the working tree;
>>>         - note where the $(cwd) was in "prefix";
>>>         - chdir to the root of the working tree;
>>> 	- prepend the "prefix" to user supplied pathspec;
>>>         - forget all the complexity and work on the whole tree.
>>>
>>> Then the "prefix" gets stripped away from the beginning of the paths when
>>> reporting.
>>>
>>> Your patch from a cursory look seems to follow that pattern, which is
>>> good.
>>
>> Thanks for the explanation.
>>
>> Yes, my code follows the pattern, except that in this case it is
>> unnecessary to chdir to the root of the working tree.
> 
> Just to make sure there is no misunderstanding. The chdir() should not be
> in the core part of the system that you may want to libify.
> 
> The above pattern was developed primarily so that older utility functions
> in the system that were written back when nobody ran git from anywhere
> other than the top level of the working tree can be easily adapted to main
> programs that can be launched from a subdirectory. The initial set-up part
> of the program is responsible for figuring out "prefix", turning relative
> paths given by the user into paths relative to the top of the working
> tree, and then chdir'ing to the top.
> 
> After all that happens, the library-ish parts of the system only have to
> deal with full paths relative to the root of the working tree. "prefix"
> comes into play when reporting the results (i.e. showing paths relative to
> user's $(cwd) in the output or in the error messages).

If I understand you correctly, the use of some API routines requires a
chdir by the caller (i.e., the surrounding application) *before* calling
into the routine.  This is certainly a bit cleaner than the library
chdiring itself, but it is still unusable in a multithreaded context.
Regardless of whether the library chdirs itself or whether it obligates
callers to chdir before calling into the library, the bottom line is
that it requires a change to global state to function correctly, and
that change can confuse other threads.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: git-fast-export is returning streams with source code inside
From: James Gregory @ 2011-08-05  9:24 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110804183218.GA15943@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

Hi Peff,

I've attached the valgrind.out file. If I'm reading the output
properly, it does look like it is suffering from a memory leak.

James

On 4 August 2011 19:32, Jeff King <peff@peff.net> wrote:
> On Thu, Aug 04, 2011 at 10:08:43AM +0100, James Gregory wrote:
>
>> Thanks for your reply. I'm afraid I won't be able to allow you access
>> to repo as it is for a business project and covered by NDAs, etc. If I
>> can do anything to help with diagnosing the problem (debug logs, more
>> excerpts from the stream, etc.) then that shouldn't be a problem.
>
> Since it seems like blob data is making it into the stream in an odd
> place, my first guess would be a memory problem. Can you try installing
> valgrind (since you are on Ubuntu, there should be a package), and then
> running:
>
>  valgrind git fast-export --all >/dev/null 2>valgrind.out
>
> and then sharing the results of valgrind.out?
>
> -Peff
>



-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21884 ~ F: +44 (0)114 22 24042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

[-- Attachment #2: valgrind.out --]
[-- Type: application/octet-stream, Size: 1746 bytes --]

==3267== Memcheck, a memory error detector
==3267== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==3267== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==3267== Command: git fast-export --all
==3267== 
==3267== Conditional jump or move depends on uninitialised value(s)
==3267==    at 0x4E39510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4E39605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4D5B02: ??? (in /usr/bin/git)
==3267==    by 0x4BB059: ??? (in /usr/bin/git)
==3267==    by 0x4BC464: ??? (in /usr/bin/git)
==3267==    by 0x4BCA29: ??? (in /usr/bin/git)
==3267==    by 0x4BCAFD: ??? (in /usr/bin/git)
==3267==    by 0x4BD161: ??? (in /usr/bin/git)
==3267==    by 0x49AEAC: ??? (in /usr/bin/git)
==3267==    by 0x4B38AA: ??? (in /usr/bin/git)
==3267==    by 0x4B422A: ??? (in /usr/bin/git)
==3267==    by 0x4A9CD2: ??? (in /usr/bin/git)
==3267== 
==3267== 
==3267== HEAP SUMMARY:
==3267==     in use at exit: 2,821,376 bytes in 6,226 blocks
==3267==   total heap usage: 356,123 allocs, 349,897 frees, 1,517,606,943 bytes allocated
==3267== 
==3267== LEAK SUMMARY:
==3267==    definitely lost: 48,016 bytes in 3 blocks
==3267==    indirectly lost: 30,226 bytes in 1,889 blocks
==3267==      possibly lost: 0 bytes in 0 blocks
==3267==    still reachable: 2,743,134 bytes in 4,334 blocks
==3267==         suppressed: 0 bytes in 0 blocks
==3267== Rerun with --leak-check=full to see details of leaked memory
==3267== 
==3267== For counts of detected and suppressed errors, rerun with: -v
==3267== Use --track-origins=yes to see where uninitialised values come from
==3267== ERROR SUMMARY: 86294 errors from 1 contexts (suppressed: 4 from 4)

^ permalink raw reply

* Re: git-fast-export is returning streams with source code inside
From: Jeff King @ 2011-08-05  9:36 UTC (permalink / raw)
  To: James Gregory; +Cc: git
In-Reply-To: <CAFC9htyFKm7NCNFvrUkxXpmj1jwatWkxrnRSEdztY4Syo+EQ-g@mail.gmail.com>

On Fri, Aug 05, 2011 at 10:24:07AM +0100, James Gregory wrote:

> I've attached the valgrind.out file. If I'm reading the output
> properly, it does look like it is suffering from a memory leak.

It looks pretty innocuous:

> ==3267== Conditional jump or move depends on uninitialised value(s)
> ==3267==    at 0x4E39510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
> ==3267==    by 0x4E39605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
> ==3267==    by 0x4D5B02: ??? (in /usr/bin/git)
> ==3267==    by 0x4BB059: ??? (in /usr/bin/git)
> ==3267==    by 0x4BC464: ??? (in /usr/bin/git)
> ==3267==    by 0x4BCA29: ??? (in /usr/bin/git)
> ==3267==    by 0x4BCAFD: ??? (in /usr/bin/git)
> ==3267==    by 0x4BD161: ??? (in /usr/bin/git)
> ==3267==    by 0x49AEAC: ??? (in /usr/bin/git)
> ==3267==    by 0x4B38AA: ??? (in /usr/bin/git)
> ==3267==    by 0x4B422A: ??? (in /usr/bin/git)
> ==3267==    by 0x4A9CD2: ??? (in /usr/bin/git)

This is a well-known false positive caused by zlib, and is nothing to
worry about.

> ==3267== LEAK SUMMARY:
> ==3267==    definitely lost: 48,016 bytes in 3 blocks
> ==3267==    indirectly lost: 30,226 bytes in 1,889 blocks

These are actual leaks, but minor.

> ==3267==      possibly lost: 0 bytes in 0 blocks
> ==3267==    still reachable: 2,743,134 bytes in 4,334 blocks

These are not really leaks, but rather things we don't bother cleaning
up since we're about to exit and let the OS reclaim memory (e.g., all of
the commit objects).

So yes, there's leaking, but it's not much. And more importantly, I was
looking not for leaks, but for memory access errors (of which there are
none, except for the zlib false positive).

So I'm not sure where to go from here. I can't reproduce the problem
locally. Is there anything else you can tell us about the problem? Does
it always happen on the same commit? If you export just that commit,
does the problem happen? Is there anything noteworthy in the contents of
that commit?

-Peff

^ permalink raw reply

* Re: git-fast-export is returning streams with source code inside
From: James Gregory @ 2011-08-05  9:54 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110805093618.GA19062@sigill.intra.peff.net>

Hi Peff,

Thanks for the feedback. I've just looked at the commit on gitweb, and
this could be the problem!

---
fixed dodgy filename
spec/blueprints/sjt_blueprint.rb	[moved from
spec/blueprints/sjt_blueprint.rb\n lead_in\n scenario\n answers {
Sham.answers_object }\n justification_selected\n
justification_unselected\n mark_scheme {
Sham.single_mark_scheme_object }\nend\nmcq_blueprint.rb with 100%
similarity]
---

I'm guessing that is where the problem lies... somehow the git
transaction has got corrupt(?)

James

On 5 August 2011 10:36, Jeff King <peff@peff.net> wrote:
> On Fri, Aug 05, 2011 at 10:24:07AM +0100, James Gregory wrote:
>
>> I've attached the valgrind.out file. If I'm reading the output
>> properly, it does look like it is suffering from a memory leak.
>
> It looks pretty innocuous:
>
>> ==3267== Conditional jump or move depends on uninitialised value(s)
>> ==3267==    at 0x4E39510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
>> ==3267==    by 0x4E39605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
>> ==3267==    by 0x4D5B02: ??? (in /usr/bin/git)
>> ==3267==    by 0x4BB059: ??? (in /usr/bin/git)
>> ==3267==    by 0x4BC464: ??? (in /usr/bin/git)
>> ==3267==    by 0x4BCA29: ??? (in /usr/bin/git)
>> ==3267==    by 0x4BCAFD: ??? (in /usr/bin/git)
>> ==3267==    by 0x4BD161: ??? (in /usr/bin/git)
>> ==3267==    by 0x49AEAC: ??? (in /usr/bin/git)
>> ==3267==    by 0x4B38AA: ??? (in /usr/bin/git)
>> ==3267==    by 0x4B422A: ??? (in /usr/bin/git)
>> ==3267==    by 0x4A9CD2: ??? (in /usr/bin/git)
>
> This is a well-known false positive caused by zlib, and is nothing to
> worry about.
>
>> ==3267== LEAK SUMMARY:
>> ==3267==    definitely lost: 48,016 bytes in 3 blocks
>> ==3267==    indirectly lost: 30,226 bytes in 1,889 blocks
>
> These are actual leaks, but minor.
>
>> ==3267==      possibly lost: 0 bytes in 0 blocks
>> ==3267==    still reachable: 2,743,134 bytes in 4,334 blocks
>
> These are not really leaks, but rather things we don't bother cleaning
> up since we're about to exit and let the OS reclaim memory (e.g., all of
> the commit objects).
>
> So yes, there's leaking, but it's not much. And more importantly, I was
> looking not for leaks, but for memory access errors (of which there are
> none, except for the zlib false positive).
>
> So I'm not sure where to go from here. I can't reproduce the problem
> locally. Is there anything else you can tell us about the problem? Does
> it always happen on the same commit? If you export just that commit,
> does the problem happen? Is there anything noteworthy in the contents of
> that commit?
>
> -Peff
>



-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21884 ~ F: +44 (0)114 22 24042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

^ permalink raw reply

* Re: git-fast-export is returning streams with source code inside
From: Jeff King @ 2011-08-05 10:30 UTC (permalink / raw)
  To: James Gregory; +Cc: git
In-Reply-To: <CAFC9htzzPQWFLGCpAP2WHhDQajfknwz1KUu6K0gvyhCVX5gyaQ@mail.gmail.com>

On Fri, Aug 05, 2011 at 10:54:29AM +0100, James Gregory wrote:

> Thanks for the feedback. I've just looked at the commit on gitweb, and
> this could be the problem!
> 
> ---
> fixed dodgy filename
> spec/blueprints/sjt_blueprint.rb	[moved from
> spec/blueprints/sjt_blueprint.rb\n lead_in\n scenario\n answers {
> Sham.answers_object }\n justification_selected\n
> justification_unselected\n mark_scheme {
> Sham.single_mark_scheme_object }\nend\nmcq_blueprint.rb with 100%
> similarity]
> ---
> 
> I'm guessing that is where the problem lies... somehow the git
> transaction has got corrupt(?)

Ah, OK. That makes sense. I can replicate your problem easily with:

  $ touch 'file with
    newline'
  $ git init
  $ git add .
  $ git commit -m foo
  $ git fast-export HEAD | git fast-import
  fatal: Unsupported command: newline

According to the fast-import manpage, fast-export should be quoting the
embedded line-feed. It looks like it isn't doing any quoting at all of
pathnames right now, which is just wrong.

Does the patch below fix your issue?

---
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9247871..bd27f08 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -16,6 +16,7 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "quote.h"
 
 static const char *fast_export_usage[] = {
 	"git fast-export [rev-list-opts]",
@@ -167,6 +168,15 @@ static int depth_first(const void *a_, const void *b_)
 	return (a->status == 'R') - (b->status == 'R');
 }
 
+static void print_path(const char *path)
+{
+	int need_quote = quote_c_style(path, NULL, NULL, 0);
+	if (need_quote)
+		quote_c_style(path, NULL, stdout, 0);
+	else
+		printf("%s", path);
+}
+
 static void show_filemodify(struct diff_queue_struct *q,
 			    struct diff_options *options, void *data)
 {
@@ -184,13 +194,18 @@ static void show_filemodify(struct diff_queue_struct *q,
 
 		switch (q->queue[i]->status) {
 		case DIFF_STATUS_DELETED:
-			printf("D %s\n", spec->path);
+			printf("D ");
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		case DIFF_STATUS_COPIED:
 		case DIFF_STATUS_RENAMED:
-			printf("%c \"%s\" \"%s\"\n", q->queue[i]->status,
-			       ospec->path, spec->path);
+			printf("%c ", q->queue[i]->status);
+			print_path(ospec->path);
+			putchar(' ');
+			print_path(spec->path);
+			putchar('\n');
 
 			if (!hashcmp(ospec->sha1, spec->sha1) &&
 			    ospec->mode == spec->mode)
@@ -205,13 +220,15 @@ static void show_filemodify(struct diff_queue_struct *q,
 			 * output the SHA-1 verbatim.
 			 */
 			if (no_data || S_ISGITLINK(spec->mode))
-				printf("M %06o %s %s\n", spec->mode,
-				       sha1_to_hex(spec->sha1), spec->path);
+				printf("M %06o %s ", spec->mode,
+				       sha1_to_hex(spec->sha1));
 			else {
 				struct object *object = lookup_object(spec->sha1);
-				printf("M %06o :%d %s\n", spec->mode,
-				       get_object_mark(object), spec->path);
+				printf("M %06o :%d ", spec->mode,
+				       get_object_mark(object));
 			}
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		default:

^ permalink raw reply related

* [PATCH] fast-export: quote paths in output
From: Jeff King @ 2011-08-05 10:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, James Gregory

Many pathnames in a fast-import stream need to be quoted. In
particular:

  1. Pathnames at the end of an "M" or "D" line need quoting
     if they contain a LF or start with double-quote.

  2. Pathnames on a "C" or "R" line need quoting as above,
     but also if they contain spaces.

For (1), we weren't quoting at all. For (2), we put
double-quotes around the paths to handle spaces, but ignored
the possibility that they would need further quoting.

This patch checks whether each pathname needs c-style
quoting, and uses it. This is slightly overkill for (1),
which doesn't actually need to quote many characters that
vanilla c-style quoting does. However, it shouldn't hurt, as
any implementation needs to be ready to handle quoted
strings anyway.

In addition to adding a test, we have to tweak a test which
blindly assumed that case (2) would always use
double-quotes, whether it needed to or not.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fast-export.c  |   31 ++++++++++++++++++++++++-------
 t/t9350-fast-export.sh |   26 +++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index becef85..9836e6b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -16,6 +16,7 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "quote.h"
 
 static const char *fast_export_usage[] = {
 	"git fast-export [rev-list-opts]",
@@ -179,6 +180,15 @@ static int depth_first(const void *a_, const void *b_)
 	return (a->status == 'R') - (b->status == 'R');
 }
 
+static void print_path(const char *path)
+{
+	int need_quote = quote_c_style(path, NULL, NULL, 0);
+	if (need_quote)
+		quote_c_style(path, NULL, stdout, 0);
+	else
+		printf("%s", path);
+}
+
 static void show_filemodify(struct diff_queue_struct *q,
 			    struct diff_options *options, void *data)
 {
@@ -196,13 +206,18 @@ static void show_filemodify(struct diff_queue_struct *q,
 
 		switch (q->queue[i]->status) {
 		case DIFF_STATUS_DELETED:
-			printf("D %s\n", spec->path);
+			printf("D ");
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		case DIFF_STATUS_COPIED:
 		case DIFF_STATUS_RENAMED:
-			printf("%c \"%s\" \"%s\"\n", q->queue[i]->status,
-			       ospec->path, spec->path);
+			printf("%c ", q->queue[i]->status);
+			print_path(ospec->path);
+			putchar(' ');
+			print_path(spec->path);
+			putchar('\n');
 
 			if (!hashcmp(ospec->sha1, spec->sha1) &&
 			    ospec->mode == spec->mode)
@@ -217,13 +232,15 @@ static void show_filemodify(struct diff_queue_struct *q,
 			 * output the SHA-1 verbatim.
 			 */
 			if (no_data || S_ISGITLINK(spec->mode))
-				printf("M %06o %s %s\n", spec->mode,
-				       sha1_to_hex(spec->sha1), spec->path);
+				printf("M %06o %s ", spec->mode,
+				       sha1_to_hex(spec->sha1));
 			else {
 				struct object *object = lookup_object(spec->sha1);
-				printf("M %06o :%d %s\n", spec->mode,
-				       get_object_mark(object), spec->path);
+				printf("M %06o :%d ", spec->mode,
+				       get_object_mark(object));
 			}
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		default:
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index f823c05..4673ac0 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -228,7 +228,7 @@ test_expect_success 'fast-export -C -C | fast-import' '
 	mkdir new &&
 	git --git-dir=new/.git init &&
 	git fast-export -C -C --signed-tags=strip --all > output &&
-	grep "^C \"file6\" \"file7\"\$" output &&
+	grep "^C file6 file7\$" output &&
 	cat output |
 	(cd new &&
 	 git fast-import &&
@@ -414,4 +414,28 @@ test_expect_success SYMLINKS 'directory becomes symlink'        '
 	(cd result && git show master:foo)
 '
 
+test_expect_success 'fast-export quotes pathnames' '
+	git init crazy-paths &&
+	(cd crazy-paths &&
+	 >"$(printf "path with\\nnewline")" &&
+	 >"path with \"quote\"" &&
+	 >"path with \\backslash" &&
+	 >"path with space" &&
+	 git add . &&
+	 git commit -m addition &&
+	 mkdir subdir &&
+	 git mv path* subdir &&
+	 git commit -m rename &&
+	 git rm -r subdir &&
+	 git commit -m deletion &&
+	 git fast-export HEAD >export.out &&
+	 git rev-list HEAD >expect &&
+	 git init result &&
+	 cd result &&
+	 git fast-import <../export.out &&
+	 git rev-list HEAD >actual &&
+	 test_cmp ../expect actual
+	)
+'
+
 test_done
-- 
1.7.6.rc0.36.gd385b.dirty

^ permalink raw reply related

* Re: [RFC/PATCH 0/5] macro-based key/value maps
From: Jeff King @ 2011-08-05 11:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20110804224354.GA27476@sigill.intra.peff.net>

On Thu, Aug 04, 2011 at 04:43:54PM -0600, Jeff King wrote:

> Well, if you like that, then here is the end-result of what the
> persistent version would look like. It's quite convenient to use, but an
> awful pain to debug.  It's done entirely in the preprocessor; I suspect
> if I wrote the code generation externally, that would be easier and more
> readable (and there are one or two places where we could be slightly
> more efficient, that are just difficult to implement via the
> preprocessor).
> 
>   [1/5]: implement generic key/value map
>   [2/5]: fast-export: use object to uint32 map instead of "decorate"
>   [3/5]: decorate: use "map" for the underlying implementation
>   [4/5]: map: implement persistent maps
>   [5/5]: implement metadata cache subsystem

Side note:

  Commits 1, 4, and 5 introduce infrastructure in the form of static
  functions and macros that contain functions that call the statics. But
  they don't actually instantiate the macro functions themselves, so
  they won't compile with -Werror (due to the "unused static" warning)
  until there is some calling code.

  That hurts bisectability a little if you compile with -Werror (you
  need to add -Wno-error=unused-function). I don't know how much we
  care.

-Peff

^ permalink raw reply

* Re: [PATCH] fast-export: quote paths in output
From: Johannes Sixt @ 2011-08-05 11:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, James Gregory
In-Reply-To: <20110805105526.GA22480@sigill.intra.peff.net>

Am 8/5/2011 12:55, schrieb Jeff King:
> +test_expect_success 'fast-export quotes pathnames' '
> +	git init crazy-paths &&
> +	(cd crazy-paths &&
> +	 >"$(printf "path with\\nnewline")" &&
> +	 >"path with \"quote\"" &&
> +	 >"path with \\backslash" &&
> +	 >"path with space" &&

Please construct the tree object using git-update-index's --cacheinfo or
--index-info rather than using actual files and git add so that the test
can pass on filesystems that do not allow newlines, quotes, or
backslashes(!) in filenames.

-- Hannes
-- 
"Atomic objects are neither active nor radioactive." --
Programming Languages -- C++, Final Committee Draft (Doc.N3092)

^ permalink raw reply

* [PATCH 0/2] bisect: cleanup formatting of git-bisect.sh
From: Jon Seymour @ 2011-08-05 11:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jon Seymour

This series is intended to be applied on top js/bisect-no-checkout.

The first patch ensures all leading whitespace is encoded with tabs, where possible.

The second patch replaces constructs of the form:

	if cond-statement; then
		then-statement
	fi

with:

	if cond-statement
	then
		then-statement
	fi	
	

Jon Seymour (2):
  bisect: cleanup whitespace errors in git-bisect.sh.
  bisect: replace "; then" with "\n<tab>*then"

 git-bisect.sh |  283 +++++++++++++++++++++++++++++----------------------------
 1 files changed, 146 insertions(+), 137 deletions(-)

-- 
1.7.6.354.g7f57d

^ permalink raw reply

* [PATCH 1/2] bisect: cleanup whitespace errors in git-bisect.sh.
From: Jon Seymour @ 2011-08-05 11:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jon Seymour
In-Reply-To: <1312543891-11079-1-git-send-email-jon.seymour@gmail.com>

All leading whitespace is now encoded with tabs.

After this patch, the following is true:

	RAW=$(cat git-bisect.sh | md5sum) &&
	ROUNDTRIP=$(cat git-bisect.sh | expand -i - | unexpand --first-only - | md5sum) &&
	LEADING=$(sed -n "/^  */p" < git-bisect.sh | wc -l) &&
	test $RAW = $ROUNDTRIP &&
	test $LEADING = 0 &&
	test -z "$(git diff -w HEAD~1 HEAD)"

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-bisect.sh |  268 ++++++++++++++++++++++++++++----------------------------
 1 files changed, 134 insertions(+), 134 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index b9c18dd..f5d9ede 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -2,27 +2,27 @@
 
 USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
-        print this long help message.
+	print this long help message.
 git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
-        reset bisect state and start bisection.
+	reset bisect state and start bisection.
 git bisect bad [<rev>]
-        mark <rev> a known-bad revision.
+	mark <rev> a known-bad revision.
 git bisect good [<rev>...]
-        mark <rev>... known-good revisions.
+	mark <rev>... known-good revisions.
 git bisect skip [(<rev>|<range>)...]
-        mark <rev>... untestable revisions.
+	mark <rev>... untestable revisions.
 git bisect next
-        find next bisection to test and check it out.
+	find next bisection to test and check it out.
 git bisect reset [<commit>]
-        finish bisection search and go back to commit.
+	finish bisection search and go back to commit.
 git bisect visualize
-        show bisect status in gitk.
+	show bisect status in gitk.
 git bisect replay <logfile>
-        replay bisection log.
+	replay bisection log.
 git bisect log
-        show bisect log.
+	show bisect log.
 git bisect run <cmd>...
-        use <cmd>... to automatically bisect.
+	use <cmd>... to automatically bisect.
 
 Please use "git help bisect" to get the full man page.'
 
@@ -55,7 +55,7 @@ bisect_autostart() {
 			# TRANSLATORS: Make sure to include [Y] and [n] in your
 			# translation. The program will only accept English input
 			# at this point.
-	    gettext "Do you want me to do it for you [Y/n]? " >&2
+			gettext "Do you want me to do it for you [Y/n]? " >&2
 			read yesno
 			case "$yesno" in
 			[Nn]*)
@@ -74,38 +74,38 @@ bisect_start() {
 	#
 	has_double_dash=0
 	for arg; do
-	    case "$arg" in --) has_double_dash=1; break ;; esac
+		case "$arg" in --) has_double_dash=1; break ;; esac
 	done
 	orig_args=$(git rev-parse --sq-quote "$@")
 	bad_seen=0
 	eval=''
 	mode=''
 	while [ $# -gt 0 ]; do
-	    arg="$1"
-	    case "$arg" in
-	    --)
-		shift
-		break
+		arg="$1"
+		case "$arg" in
+		--)
+			shift
+			break
 		;;
-	    --no-checkout)
-		mode=--no-checkout
-		shift ;;
-	    --*)
-		die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
-	    *)
-		rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-		    test $has_double_dash -eq 1 &&
+		--no-checkout)
+			mode=--no-checkout
+			shift ;;
+		--*)
+			die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
+		*)
+			rev=$(git rev-parse -q --verify "$arg^{commit}") || {
+			    test $has_double_dash -eq 1 &&
 			die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-		    break
-		}
-		case $bad_seen in
-		0) state='bad' ; bad_seen=1 ;;
-		*) state='good' ;;
+			    break
+			}
+			case $bad_seen in
+			0) state='bad' ; bad_seen=1 ;;
+			*) state='good' ;;
+			esac
+			eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
+			shift
+			;;
 		esac
-		eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
-		shift
-		;;
-	    esac
 	done
 
 	#
@@ -207,18 +207,18 @@ check_expected_revs() {
 }
 
 bisect_skip() {
-        all=''
+	all=''
 	for arg in "$@"
 	do
-	    case "$arg" in
-            *..*)
-		revs=$(git rev-list "$arg") || die "$(eval_gettext "Bad rev input: \$arg")" ;;
-            *)
-                revs=$(git rev-parse --sq-quote "$arg") ;;
-	    esac
-            all="$all $revs"
-        done
-        eval bisect_state 'skip' $all
+		case "$arg" in
+		*..*)
+			revs=$(git rev-list "$arg") || die "$(eval_gettext "Bad rev input: \$arg")" ;;
+		*)
+			revs=$(git rev-parse --sq-quote "$arg") ;;
+		esac
+		all="$all $revs"
+	done
+	eval bisect_state 'skip' $all
 }
 
 bisect_state() {
@@ -316,7 +316,7 @@ bisect_next() {
 	git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
 	res=$?
 
-        # Check if we should exit because bisection is finished
+	# Check if we should exit because bisection is finished
 	test $res -eq 10 && exit 0
 
 	# Check for an error in the bisection process
@@ -355,12 +355,12 @@ bisect_reset() {
 	case "$#" in
 	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
 	1) git rev-parse --quiet --verify "$1^{commit}" > /dev/null || {
-	       invalid="$1"
-	       die "$(eval_gettext "'\$invalid' is not a valid commit")"
-	   }
-	   branch="$1" ;;
+			invalid="$1"
+			die "$(eval_gettext "'\$invalid' is not a valid commit")"
+		}
+		branch="$1" ;;
 	*)
-	    usage ;;
+		usage ;;
 	esac
 	if ! test -f "$GIT_DIR/BISECT_HEAD"
 	then
@@ -418,65 +418,65 @@ bisect_replay () {
 }
 
 bisect_run () {
-    bisect_next_check fail
-
-    while true
-    do
-      command="$@"
-      eval_gettext "running \$command"; echo
-      "$@"
-      res=$?
-
-      # Check for really bad run error.
-      if [ $res -lt 0 -o $res -ge 128 ]; then
-	  (
-	    eval_gettext "bisect run failed:
+	bisect_next_check fail
+
+	while true
+	do
+		command="$@"
+		eval_gettext "running \$command"; echo
+		"$@"
+		res=$?
+
+		# Check for really bad run error.
+		if [ $res -lt 0 -o $res -ge 128 ]; then
+			(
+				eval_gettext "bisect run failed:
 exit code \$res from '\$command' is < 0 or >= 128" &&
-	    echo
-	  ) >&2
-	  exit $res
-      fi
-
-      # Find current state depending on run success or failure.
-      # A special exit code of 125 means cannot test.
-      if [ $res -eq 125 ]; then
-	  state='skip'
-      elif [ $res -gt 0 ]; then
-	  state='bad'
-      else
-	  state='good'
-      fi
-
-      # We have to use a subshell because "bisect_state" can exit.
-      ( bisect_state $state > "$GIT_DIR/BISECT_RUN" )
-      res=$?
-
-      cat "$GIT_DIR/BISECT_RUN"
-
-      if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
-		> /dev/null; then
-	  (
-	      gettext "bisect run cannot continue any more" &&
-	      echo
-	  ) >&2
-	  exit $res
-      fi
-
-      if [ $res -ne 0 ]; then
-	  (
-	      eval_gettext "bisect run failed:
+				echo
+			) >&2
+			exit $res
+		fi
+
+		# Find current state depending on run success or failure.
+		# A special exit code of 125 means cannot test.
+		if [ $res -eq 125 ]; then
+			state='skip'
+		elif [ $res -gt 0 ]; then
+			state='bad'
+		else
+			state='good'
+		fi
+
+		# We have to use a subshell because "bisect_state" can exit.
+		( bisect_state $state > "$GIT_DIR/BISECT_RUN" )
+		res=$?
+
+		cat "$GIT_DIR/BISECT_RUN"
+
+		if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
+			> /dev/null; then
+			(
+				gettext "bisect run cannot continue any more" &&
+				echo
+			) >&2
+			exit $res
+		fi
+
+		if [ $res -ne 0 ]; then
+			(
+				eval_gettext "bisect run failed:
 'bisect_state \$state' exited with error code \$res" &&
-	      echo
-	  ) >&2
-	  exit $res
-      fi
+				echo
+			) >&2
+			exit $res
+		fi
 
-      if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
-	  gettext "bisect run success"; echo
-	  exit 0;
-      fi
+		if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+			gettext "bisect run success"; echo
+			exit 0;
+		fi
 
-    done
+	done
 }
 
 bisect_log () {
@@ -486,33 +486,33 @@ bisect_log () {
 
 case "$#" in
 0)
-    usage ;;
+	usage ;;
 *)
-    cmd="$1"
-    shift
-    case "$cmd" in
-    help)
-        git bisect -h ;;
-    start)
-        bisect_start "$@" ;;
-    bad|good)
-        bisect_state "$cmd" "$@" ;;
-    skip)
-        bisect_skip "$@" ;;
-    next)
-        # Not sure we want "next" at the UI level anymore.
-        bisect_next "$@" ;;
-    visualize|view)
-	bisect_visualize "$@" ;;
-    reset)
-        bisect_reset "$@" ;;
-    replay)
-	bisect_replay "$@" ;;
-    log)
-	bisect_log ;;
-    run)
-        bisect_run "$@" ;;
-    *)
-        usage ;;
-    esac
+	cmd="$1"
+	shift
+	case "$cmd" in
+	help)
+		git bisect -h ;;
+	start)
+		bisect_start "$@" ;;
+	bad|good)
+		bisect_state "$cmd" "$@" ;;
+	skip)
+		bisect_skip "$@" ;;
+	next)
+		# Not sure we want "next" at the UI level anymore.
+		bisect_next "$@" ;;
+	visualize|view)
+		bisect_visualize "$@" ;;
+	reset)
+		bisect_reset "$@" ;;
+	replay)
+		bisect_replay "$@" ;;
+	log)
+		bisect_log ;;
+	run)
+		bisect_run "$@" ;;
+	*)
+		usage ;;
+	esac
 esac
-- 
1.7.6.354.g7f57d

^ permalink raw reply related

* [PATCH 2/2] bisect: replace "; then" with "\n<tab>*then"
From: Jon Seymour @ 2011-08-05 11:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jon Seymour
In-Reply-To: <1312543891-11079-1-git-send-email-jon.seymour@gmail.com>

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-bisect.sh |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index f5d9ede..1f3c46d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -198,7 +198,8 @@ is_expected_rev() {
 
 check_expected_revs() {
 	for _rev in "$@"; do
-		if ! is_expected_rev "$_rev"; then
+		if ! is_expected_rev "$_rev"
+		then
 			rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
 			rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
 			return
@@ -331,7 +332,8 @@ bisect_visualize() {
 	if test $# = 0
 	then
 		if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
-		   type gitk >/dev/null 2>&1; then
+		   type gitk >/dev/null 2>&1
+		then
 			set gitk
 		else
 			set git log
@@ -400,7 +402,8 @@ bisect_replay () {
 	while read git bisect command rev
 	do
 		test "$git $bisect" = "git bisect" -o "$git" = "git-bisect" || continue
-		if test "$git" = "git-bisect"; then
+		if test "$git" = "git-bisect"
+		then
 			rev="$command"
 			command="$bisect"
 		fi
@@ -428,7 +431,8 @@ bisect_run () {
 		res=$?
 
 		# Check for really bad run error.
-		if [ $res -lt 0 -o $res -ge 128 ]; then
+		if [ $res -lt 0 -o $res -ge 128 ]
+		then
 			(
 				eval_gettext "bisect run failed:
 exit code \$res from '\$command' is < 0 or >= 128" &&
@@ -439,9 +443,11 @@ exit code \$res from '\$command' is < 0 or >= 128" &&
 
 		# Find current state depending on run success or failure.
 		# A special exit code of 125 means cannot test.
-		if [ $res -eq 125 ]; then
+		if [ $res -eq 125 ]
+		then
 			state='skip'
-		elif [ $res -gt 0 ]; then
+		elif [ $res -gt 0 ]
+		then
 			state='bad'
 		else
 			state='good'
@@ -454,7 +460,8 @@ exit code \$res from '\$command' is < 0 or >= 128" &&
 		cat "$GIT_DIR/BISECT_RUN"
 
 		if sane_grep "first bad commit could be any of" "$GIT_DIR/BISECT_RUN" \
-			> /dev/null; then
+			> /dev/null
+		then
 			(
 				gettext "bisect run cannot continue any more" &&
 				echo
@@ -462,7 +469,8 @@ exit code \$res from '\$command' is < 0 or >= 128" &&
 			exit $res
 		fi
 
-		if [ $res -ne 0 ]; then
+		if [ $res -ne 0 ]
+		then
 			(
 				eval_gettext "bisect run failed:
 'bisect_state \$state' exited with error code \$res" &&
@@ -471,7 +479,8 @@ exit code \$res from '\$command' is < 0 or >= 128" &&
 			exit $res
 		fi
 
-		if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null; then
+		if sane_grep "is the first bad commit" "$GIT_DIR/BISECT_RUN" > /dev/null
+		then
 			gettext "bisect run success"; echo
 			exit 0;
 		fi
-- 
1.7.6.354.g7f57d

^ permalink raw reply related

* Re: [PATCH] rebase -i: fix has_action
From: Sverre Rabbelier @ 2011-08-05 12:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Noe Rubinstein, git
In-Reply-To: <7vliv93r9g.fsf@alter.siamese.dyndns.org>

Heya,

On Thu, Aug 4, 2011 at 21:34, Junio C Hamano <gitster@pobox.com> wrote:
>        has_action () {
>          test -s "$1"
>        }

>        has_action () {
>          sane_grep -v -e '^#' -e '^[   ]*$' "$1" >/dev/null
>        }

I think the former more correctly checks what the function name
implies, is there any downside to that which makes you suggest this
second approach?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] rebase -i: fix has_action
From: Johannes Sixt @ 2011-08-05 12:46 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Junio C Hamano, Noe Rubinstein, git
In-Reply-To: <CAGdFq_j2wRw-gB109VypZkG1u=fm7yynkn2-Gu8AzNpVOrun8w@mail.gmail.com>

Am 8/5/2011 14:36, schrieb Sverre Rabbelier:
> On Thu, Aug 4, 2011 at 21:34, Junio C Hamano <gitster@pobox.com> wrote:
>>        has_action () {
>>          test -s "$1"
>>        }
> 
>>        has_action () {
>>          sane_grep -v -e '^#' -e '^[   ]*$' "$1" >/dev/null
>>        }
> 
> I think the former more correctly checks what the function name
> implies, is there any downside to that which makes you suggest this
> second approach?

Yes. There might be editors where it is difficult to edit a non-empty file
so that it becomes empty. I recall this was a problem for me with vi
before I got intimately acquainted with it.

-- Hannes

^ permalink raw reply

* [PATCH] misc-sh: fix up whitespace in some other .sh files.
From: Jon Seymour @ 2011-08-05 13:31 UTC (permalink / raw)
  To: git; +Cc: Jon Seymour

I found that the patched 4 files were different when this
filter is applied.

	expand -i | unexpand --first-only

This patch contains the corrected files.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 generate-cmdlist.sh   |    4 ++--
 git-filter-branch.sh  |   12 ++++++------
 git-mergetool--lib.sh |    6 +++---
 git-rebase.sh         |    2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 3ef4861..1093ef4 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -15,8 +15,8 @@ do
      sed -n '
      /^NAME/,/git-'"$cmd"'/H
      ${
-            x
-            s/.*git-'"$cmd"' - \(.*\)/  {"'"$cmd"'", "\1"},/
+	    x
+	    s/.*git-'"$cmd"' - \(.*\)/  {"'"$cmd"'", "\1"},/
 	    p
      }' "Documentation/git-$cmd.txt"
 done
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 962a93b..0d399aa 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -12,7 +12,7 @@
 
 functions=$(cat << \EOF
 warn () {
-        echo "$*" >&2
+	echo "$*" >&2
 }
 
 map()
@@ -98,11 +98,11 @@ set_ident () {
 }
 
 USAGE="[--env-filter <command>] [--tree-filter <command>]
-            [--index-filter <command>] [--parent-filter <command>]
-            [--msg-filter <command>] [--commit-filter <command>]
-            [--tag-name-filter <command>] [--subdirectory-filter <directory>]
-            [--original <namespace>] [-d <directory>] [-f | --force]
-            [<rev-list options>...]"
+	    [--index-filter <command>] [--parent-filter <command>]
+	    [--msg-filter <command>] [--commit-filter <command>]
+	    [--tag-name-filter <command>] [--subdirectory-filter <directory>]
+	    [--original <namespace>] [-d <directory>] [-f | --force]
+	    [<rev-list options>...]"
 
 OPTIONS_SPEC=
 . git-sh-setup
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 91f90ac..9a89e8f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -79,7 +79,7 @@ get_merge_tool_cmd () {
 	fi
 	if diff_mode; then
 		echo "$(git config difftool.$merge_tool.cmd ||
-		        git config mergetool.$merge_tool.cmd)"
+			git config mergetool.$merge_tool.cmd)"
 	else
 		echo "$(git config mergetool.$merge_tool.cmd)"
 	fi
@@ -419,7 +419,7 @@ get_merge_tool_path () {
 	fi
 	if diff_mode; then
 		merge_tool_path=$(git config difftool."$merge_tool".path ||
-		                  git config mergetool."$merge_tool".path)
+				  git config mergetool."$merge_tool".path)
 	else
 		merge_tool_path=$(git config mergetool."$merge_tool".path)
 	fi
@@ -429,7 +429,7 @@ get_merge_tool_path () {
 	if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
 	! type "$merge_tool_path" > /dev/null 2>&1; then
 		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
-		         "'$merge_tool_path'"
+			 "'$merge_tool_path'"
 		exit 1
 	fi
 	echo "$merge_tool_path"
diff --git a/git-rebase.sh b/git-rebase.sh
index 266a4c1..6759702 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -22,7 +22,7 @@ currently checked out branch is used.
 
 Example:       git-rebase master~1 topic
 
-        A---B---C topic                   A'\''--B'\''--C'\'' topic
+	A---B---C topic                   A'\''--B'\''--C'\'' topic
        /                   -->           /
   D---E---F---G master          D---E---F---G master
 '
-- 
1.7.6.360.geebb0f

^ permalink raw reply related

* Re: [PATCH] rebase -i: fix has_action
From: Andrew Wong @ 2011-08-05 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Noe Rubinstein, git
In-Reply-To: <7vliv93r9g.fsf@alter.siamese.dyndns.org>

On 08/04/2011 03:34 PM, Junio C Hamano wrote:
> How about removing the unnecessary negation from the logic and directly
> ask what we really want to know?
>
> That is, "Do we have a line that is _not_ comment?"
>
> 	has_action () {
>           sane_grep -v -e '^#' -e '^[   ]*$' "$1" >/dev/null
> 	}
How about also including comments that begins with spaces? i.e.

    has_action () {
        sane_grep -v -e '^[   ]*#' -e '^[   ]*$' "$1" >/dev/null
    }

Also, is [   ] supposed to be a space and a hard tab? They just seem to
be three spaces in my email. We might need to watch out for the hard tab
getting expanded into spaces somewhere during the email process,
especially when applying the patch from email into code.

Andrew

^ permalink raw reply

* What you can throw (on a Friday)
From: Steffen Daode Nurpmeso @ 2011-08-05 14:47 UTC (permalink / raw)
  To: git
In-Reply-To: <7vliv93r9g.fsf@alter.siamese.dyndns.org>

@ Junio C Hamano <gitster@pobox.com> wrote (2011-08-04 21:34+0200):
> Noe Rubinstein <nrubinstein@proformatique.com> writes: [...]
> If a user fat-fingers an unnecessary [...]

Heh.  Note that one of the first mails i've received from this
list was the one which transported the annoyed gasp from
the-one-who-was-born-right-next-door-to-where-the-elks-live.
About scattered non-breaking spaces (U+A0).

Unfortunately that patch series has been thrown away!  And so
i think you underestimate the problem.  On my german keyboard,
f.e., i need to press ALT for all of these: []|{} (ALT + [5-9]).
!!!  Well, there *are* days where my thumb is fast enough to leave
ALT before i hit SPC ...  (But today it yet caused distress.)

After the second scissor i'll append my current pre-commit (and
thus pre-applypatch) (for nothing except to show that it's
a problem people really have to deal with).  But i already thought
about resurrecting your patch-series and reduce it to only the
whitespace check.  Because, being able to say

    exec git diff-index --check --cached $against --

instead would be much easier, because then you could simply state
and request from contributors "please adhere to the whitespace
policy of this project:"

    whitespace = trailing-space,tabwidth=4,tab-in-indent,yell-on-nbsp

> Hmm?

Well and while fooling around and getting more familiar with
git(1) i stumbled over some things which might be caused by bad
control flow instead of being desired behaviour.  After the first
scissor there is a test shell script which reproduces them.

Thanks for git(1) beside that, plastic dishes also break ...
Nice weekend all of you.

--Steffen
Ciao, sdaoden(*)(gmail.com)
ASCII ribbon campaign           ( ) More nuclear fission plants
  against HTML e-mail            X    can serve more coloured
    and proprietary attachments / \     and sounding animations

-- >8 --
#!/bin/sh

error() {
	echo >&2 Error: $*
}

add_file() {
    local f=$1
    echo $f > $f
    git add $f
    git commit -qm $f
}

origin() {
    rm -rf origin
    mkdir origin
    cd origin
    git init -q
    add_file eins
    add_file zwei
    add_file drei
    add_file vier
    add_file fuenf
    git checkout -qb devel
    add_file devel-one
    add_file devel-two
    git checkout -q master
    git tag -m vT1 vT1 HEAD
    cd ..
}

badbad_tagopt() {
    echo
    echo
    echo '1. echo git fetch will --prune away branches if --tags is set'
    echo '   (even if done so through remote.XY.tagopt config).'
    echo '   But nice: it works well again if --all is also given.'
    echo
	work() {
        echo - Am using fetch -q --prune $1 $2
		rm -rf tr1
		mkdir tr1
		cd tr1
		git init -q
		git remote add -t master -t devel -m master origin ../origin

		git fetch -q --prune $1 $2

		git branch -a | grep -F 'origin/master' || error no master branch
		git branch -a | grep -F 'origin/devel' || error no devel branch
		cd ..
		rm -rf tr1
	}


	work '' ''
	work '--tags' ''
    work '--tags' '--all'
}

lazy_ref() {
    echo
    echo
    echo 2. If you do drop/there is no remote.XY.fetch of a branch,
    echo '   but configure the remote.XY.push entry, then after'
    echo '   git push the local ref is not updated, even though'
    echo '   the push succeeded and correctly updated the target repo.'
    echo
	work() {
        echo - remote.origin.fetch will include devel branch: $#
        cp -R origin origin.save
		rm -rf tr1
		mkdir tr1
		cd tr1
		git init -q
		git remote add -t master -m master -t devel origin ../origin
        git config --local remote.origin.push \
                           +refs/heads/master:refs/heads/master
        git config --local --add remote.origin.push \
                            +refs/heads/devel:refs/heads/devel
		git fetch -q --prune
        git checkout -q master
        git checkout -q devel
        add_file test-repo-devel-branch-file

        test $# == 0 &&
            git config --local --replace-all remote.origin.fetch \
                                 +refs/heads/master:refs/remotes/origin/master
        git push -q origin

        x=$(git show-ref --hash devel | sort -u | awk '{++l} END {print l}')
        test $x == 1 || error 'local-ref mismatch'
		cd ..
		rm -rf tr1 origin
        mv origin.save origin
	}

	work YesPlease
	work
}

cd $TMPDIR
mkdir workdir
cd workdir

origin

badbad_tagopt
lazy_ref

cd ..
rm -rf workdir
exit 0
-- >8 --
#!/bin/sh
#@ git(1) pre-commit hook for dummies

#if git rev-parse --verify HEAD >/dev/null 2>&1
#then
    against=HEAD
#else
    # Initial commit: diff against an empty tree object
#   against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
#fi

# Oh no, unfortunately not: exec git diff-index --check --cached $against --
git diff --cached $against | perl -e '
    # XXX 1st version, may not be able to swallow all possible diff output yet
    my ($estat, $l, $fname) = (0, undef, undef);

    for (;;) { last if stdin() =~ /^diff/o; }
    for (;;) { head(); hunk(); }

    sub stdin {
        $l = <STDIN>;
        exit($estat) unless $l;
        chomp($l);
        return $l;
    }

    sub head {
        # Skip anything, including options and entire rename and delete diffs,
        # until we see the ---/+++ line pair
        for (;;) {
            last if $l =~ /^---/o;
            stdin();
        }

        stdin();
        die "head, 1.: cannot parse diff!" unless $l =~ /^\+\+\+ /o;
        $fname = substr($l, 4);
        $fname = substr($fname, 2) if $fname =~ /^b\//o;
    }

    sub hunk() {
        stdin();
        die "hunk, 1.: cannot parse diff!" unless $l =~ /^@@ /o;
JHUNK:
        # regex shamelessly stolen from git(1), and modified
        $l =~ /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/;
        my $lno = $1 - 1;

        for (;;) {
            stdin();
            return if $l =~ /^diff/o;       # Different file?
            goto JHUNK if $l =~ /^@@ /o;    # Same file, different hunk?
            next if $l =~ /^-/o;            # Ignore removals

            ++$lno;
            next if $l =~ /^ /o;
            $l = substr($l, 1);

            if (index($l, "\xA0") != -1) {
                $estat = 1;
                print "$fname:$lno: non-breaking space (NBSP, U+A0).\n";
            }
            if ($l =~ /\s+$/o) {
                $estat = 1;
                print "$fname:$lno: trailing whitespace.\n";
            }
            if ($l =~ /^(\s+)/o && $1 =~ /\x09/o) {
                $estat = 1;
                print "$fname:$lno: tabulator in indent.\n";
            }
        }
    }
    '

^ permalink raw reply

* Re: [RFC 4/6] git-check-attr: Normalize paths
From: Junio C Hamano @ 2011-08-05 15:02 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git
In-Reply-To: <4E3B8CAA.4030002@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> If I understand you correctly, the use of some API routines requires a
> chdir by the caller (i.e., the surrounding application) *before* calling
> into the routine.  This is certainly a bit cleaner than the library
> chdiring itself, but it is still unusable in a multithreaded context.

Why?

Presumably you know what your threads are doing, so if you take input from
the end user after you started the environment, you will be doing the
prefix discovery and pathspec prefixing on the entry and prefix stripping
upon output but do not have to (and should not be doing) chdir at all.

^ permalink raw reply

* Re: [RFC/PATCH 0/5] macro-based key/value maps
From: René Scharfe @ 2011-08-05 15:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20110805110302.GA23619@sigill.intra.peff.net>

Am 05.08.2011 13:03, schrieb Jeff King:
>   Commits 1, 4, and 5 introduce infrastructure in the form of static
>   functions and macros that contain functions that call the statics. But
>   they don't actually instantiate the macro functions themselves, so
>   they won't compile with -Werror (due to the "unused static" warning)
>   until there is some calling code.
> 
>   That hurts bisectability a little if you compile with -Werror (you
>   need to add -Wno-error=unused-function). I don't know how much we
>   care.

I don't know either, but you could avoid the issue by adding a test-maps
command in the first patch and exercising the new functionality a bit.

René

^ permalink raw reply

* [PATH 0/2] On Cygwin support both UNIX and DOS style path-names
From: Pascal Obry @ 2011-08-05 16:10 UTC (permalink / raw)
  To: git

This is the second version of this serie after Theo review. It includes
the patch from Theo first and mine after.

Let me know if it looks ok. Thanks.

^ permalink raw reply

* [PATCH 1/2] git-compat-util: add generic find_last_dir_sep that respects is_dir_sep
From: Pascal Obry @ 2011-08-05 16:10 UTC (permalink / raw)
  To: git; +Cc: Theo Niessink
In-Reply-To: <1312560614-20772-1-git-send-email-pascal@obry.net>

From: Theo Niessink <theo@taletn.com>

Move MinGW's find_last_dir_sep to git-compat-util.h, so it can also be used
on other platforms that define is_dir_sep, e.g. Cygwin.

Signed-off-by: Theo Niessink <theo@taletn.com>
Acked-by: Pascal Obry <pascal@obry.net>
---
 compat/mingw.h    |    9 ---------
 git-compat-util.h |   12 ++++++++++++
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index ce9dd98..547568b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -300,15 +300,6 @@ int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format
 
 #define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
 #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
-static inline char *mingw_find_last_dir_sep(const char *path)
-{
-	char *ret = NULL;
-	for (; *path; ++path)
-		if (is_dir_sep(*path))
-			ret = (char *)path;
-	return ret;
-}
-#define find_last_dir_sep mingw_find_last_dir_sep
 #define PATH_SEP ';'
 #define PRIuMAX "I64u"
 
diff --git a/git-compat-util.h b/git-compat-util.h
index ddfbf77..c2c94cd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -211,6 +211,18 @@ extern char *gitbasename(char *);
 #define has_dos_drive_prefix(path) 0
 #endif
 
+#if !defined(find_last_dir_sep) && defined(is_dir_sep)
+static inline char *compat_find_last_dir_sep(const char *path)
+{
+	char *ret = NULL;
+	for (; *path; ++path)
+		if (is_dir_sep(*path))
+			ret = (char *)path;
+	return ret;
+}
+#define find_last_dir_sep compat_find_last_dir_sep
+#endif
+
 #ifndef is_dir_sep
 #define is_dir_sep(c) ((c) == '/')
 #endif
-- 
1.7.6.404.g5d2fc

^ permalink raw reply related

* [PATCH 2/2] On Cygwin support both UNIX and DOS style path-names
From: Pascal Obry @ 2011-08-05 16:10 UTC (permalink / raw)
  To: git; +Cc: Pascal Obry
In-Reply-To: <1312560614-20772-1-git-send-email-pascal@obry.net>

In fact Cygwin supports both, so make Git agree with this.
The failing case is when a file is committed in a sub-dir of the
repository using a log message from a file specified with a DOS
style path-name. To reproduce:

   $ cd src
   $ git commit -F c:\tmp\log.txt file.c
   fatal: could not read log file 'src/c:\tmp\log.txt': No such file \
   or directory.

Signed-off-by: Pascal Obry <pascal@obry.net>
---
 compat/cygwin.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/compat/cygwin.h b/compat/cygwin.h
index a3229f5..de9737c 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -7,3 +7,6 @@ extern stat_fn_t cygwin_lstat_fn;
 
 #define stat(path, buf) (*cygwin_stat_fn)(path, buf)
 #define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)
+
+#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
+#define is_dir_sep(c) ((c) == '/' || (c) == '\\')
-- 
1.7.6.404.g5d2fc

^ permalink raw reply related

* Re: [PATCH] rebase -i: fix has_action
From: Junio C Hamano @ 2011-08-05 16:59 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Noe Rubinstein, git
In-Reply-To: <CAGdFq_j2wRw-gB109VypZkG1u=fm7yynkn2-Gu8AzNpVOrun8w@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Thu, Aug 4, 2011 at 21:34, Junio C Hamano <gitster@pobox.com> wrote:
>>        has_action () {
>>          test -s "$1"
>>        }
>
>>        has_action () {
>>          sane_grep -v -e '^#' -e '^[   ]*$' "$1" >/dev/null
>>        }
>
> I think the former more correctly checks what the function name
> implies, is there any downside to that which makes you suggest this
> second approach?

I vaguely recall the original reason we didn't do the most straightforward
thing was something like what J6t said already.

As we are not interested in _adding_ new feature, I would say that,
strictly speaking, this *should* become a two-patch series whose first one
uses

	sane_grep -v -e '^#' -e '^$' "$1" >/dev/null

that is, "do we have anything aside from comments and blanks?", which is
the original semantics, with Noe's "safety" change as the second patch in
the series that uses

	sane_grep -v -e '^#' -e '^[	 ]*$' "$1" >/dev/null

to say "let's count a line that solely consists of whitespaces also as a
blank".

But of course in practice it can and should be just a single patch that 
squashes these two "conceptually separate" steps.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox