Git development
 help / color / mirror / Atom feed
* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Wincent Colaiuta @ 2007-11-02  8:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steffen Prohaska, git
In-Reply-To: <7vlk9jgeee.fsf@gitster.siamese.dyndns.org>

El 31/10/2007, a las 22:31, Junio C Hamano escribió:

> Wrong.  push is a mirror of fetch and does not do _any_
> integration.  It is just a safe (because it insists on
> fast-forward) propagation mechanism.  Your integration still
> happens with pull (actually, shared repository people seem to
> prefer "fetch + rebase" over "pull" which is "fetch + merge").


Of course, it's too late too change now, but it would be nice if the  
mirror of "fetch" were "send". (I know it's been commented in the past  
that the fact that "push" and "pull" aren't mirror operations has  
surprised quite a few people.)

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Junio C Hamano @ 2007-11-02  7:52 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Andreas Ericsson, git
In-Reply-To: <63FCD695-B952-4624-854C-0F1C662D94D1@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> On Nov 1, 2007, at 9:18 PM, Junio C Hamano wrote:
>
>> The context of this "forced" is that you say (in the following
>> paragraph) the user's main objective was to "push", but I do not
>> think "to push" is ever the main objective.
>
> Right. I should probably describe a bit more of the context.

Boring ;-)

> We have a shared branch for a group of developer who are located
> ...
> In this setting a user really want to push. Because only then
> the code will be tested and available for all others. ...

Pretty much expected, sane, and unsurprising.  Then you are in
the first category I quoted, and...

>>  - If it is to give integrated result for others to work further
>>    on, then you need to resolve before being able to achieve
>>    that goal.  There is no escaping from it.

... it still holds that what the developer wants to do is not
just "to push", but "to push after making sure what he is going
to push is in a good enough shape to be pushed".  Your _workflow_
is forcing to integrate right away before pushing; don't blame
git for this.

>>  - On the other hand, if it is to show what you did as early as
>>    possible in a working shape, and if the updated shared
>>    repository has changes from somebody else that conflicts you,
>>    in a CVS/SVN style shared workflow, there is no way for you
>>    to show what you did in isolation.  If you try to follow that
>>    model in git and insist pushing to the same branch, then you
>>    are forced to resolve first.
>>
>>    But you do not have to.  You could push out to another new
>>    branch, and say "Here is how you could do it, although this
>>    is based on an older codebase and conflicts with what
>>    recently happened to the tip".  You could even ask other
>>    party whose changes conflict with yours to help with the
>>    merge by saying "I pushed it out, you are more familiar with
>>    that area of the code and with your changes near the tip of
>>    the trunk, so could you merge it and push out the result?"
>
> ... I know we could use git to establish a more complex workflow
> that would give better guarantees on the published branches.

Don't get me wrong.  You do not always have to use the "push to
a side branch and ask for help from others", but git opens the
door for you to do so more conveniently, rather than strictly
sticking to the CVS workflow.    I re-quoted the whole "On the
other hand" part because I think this is something not often
done by people with CVS background --- with CVS you can do
exactly the same thing but it is too cumbersome and people don't
do so in practice.  With git, such an interaction is not just
possible but is a very natural thing to do.

Your more advanced people can be the first ones to employ this
"new communication medium" to help work better among them.  You
do not have to force the "side communication" as an official
part of workflow to the whole group.

SCM is just a tool to help developer communication.  Use it
wisely.

> We haven't figured out much more of our workflow. The first
> milestone is to migrate from CVS to git continuing to use a
> CVS-style workflow.

I think that is an interesting admission.  As somebody else on
the thread already said, if you are sticking to CVS workflow,
there are things that can and cannot be naturally done with
git.  Don't break git when you hit the situation in the latter
category without understanding how the world works.

> error: remote 'refs/heads/master' is ahead of local 'refs/heads/
> master'. Use --verbose for more details.

I'd rather have "Read section XXX of the user's guide".

^ permalink raw reply

* [PATCH 2/2] git-diff: complain about >=8 consecutive spaces in initial indent
From: Junio C Hamano @ 2007-11-02  7:38 UTC (permalink / raw)
  To: git; +Cc: Brian Downing
In-Reply-To: <7vwst15ceq.fsf@gitster.siamese.dyndns.org>

This introduces a new whitespace error type, "indent-with-non-tab".
The error is about starting a line with 8 or more SP, instead of
indenting it with a HT.

This is not enabled by default, as some projects employ an
indenting policy to use only SPs and no HTs.

The kernel folks and git contributors may want to enable this
detection with:

	[core]
		whitespace = indent-with-non-tab

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Obviously, it is too late at night for me to do the same for
   git-apply, but it's Ok as I know there are capable interested
   parties on the list.  Hint, hint...

 cache.h  |    1 +
 config.c |    1 +
 diff.c   |   14 ++++++++++++--
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index a6e5988..3f42827 100644
--- a/cache.h
+++ b/cache.h
@@ -608,6 +608,7 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
  */
 #define WS_TRAILING_SPACE	01
 #define WS_SPACE_BEFORE_TAB	02
+#define WS_INDENT_WITH_NON_TAB	04
 #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
 extern unsigned whitespace_rule;
 
diff --git a/config.c b/config.c
index ffb418c..d5b9766 100644
--- a/config.c
+++ b/config.c
@@ -252,6 +252,7 @@ static struct whitespace_rule {
 } whitespace_rule_names[] = {
 	{ "trailing-space", WS_TRAILING_SPACE },
 	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
+	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
 };
 
 static unsigned parse_whitespace_rule(const char *string)
diff --git a/diff.c b/diff.c
index 2112353..6bb902f 100644
--- a/diff.c
+++ b/diff.c
@@ -502,8 +502,11 @@ static void emit_line_with_ws(int nparents,
 	int i;
 	int tail = len;
 	int need_highlight_leading_space = 0;
-	/* The line is a newly added line.  Does it have funny leading
-	 * whitespaces?  In indent, SP should never precede a TAB.
+	/*
+	 * The line is a newly added line.  Does it have funny leading
+	 * whitespaces?  In indent, SP should never precede a TAB.  In
+	 * addition, under "indent with non tab" rule, there should not
+	 * be more than 8 consecutive spaces.
 	 */
 	for (i = col0; i < len; i++) {
 		if (line[i] == '\t') {
@@ -517,6 +520,13 @@ static void emit_line_with_ws(int nparents,
 		else
 			break;
 	}
+	if ((whitespace_rule & WS_INDENT_WITH_NON_TAB) &&
+	    0 <= last_space_in_indent &&
+	    last_tab_in_indent < 0 &&
+	    8 <= (i - col0)) {
+		last_tab_in_indent = i;
+		need_highlight_leading_space = 1;
+	}
 	fputs(set, stdout);
 	fwrite(line, col0, 1, stdout);
 	fputs(reset, stdout);
-- 
1.5.3.5.1452.ga93d

^ permalink raw reply related

* [PATCH 1/2] War on whitespace: first, a bit of retreat.
From: Junio C Hamano @ 2007-11-02  7:34 UTC (permalink / raw)
  To: git; +Cc: Brian Downing

This introduces core.whitespace configuration variable that lets
you specify the definition of "whitespace error".

Currently there are two kinds of whitespace errors defined:

 * trailing-space: trailing whitespaces at the end of the line.

 * space-before-tab: a SP appears immediately before HT in the
   indent part of the line.

You can specify the desired types of errors to be detected by
listing their names (unique abbreviations are accepted)
separated by comma.  By default, these two errors are always
detected, as that is the traditional behaviour.  You can disable
detection of a particular type of error by prefixing a '-' in
front of the name of the error, like this:

	[core]
		whitespace = -trailing-space

This patch teaches the code to output colored diff with
DIFF_WHITESPACE color to highlight the detected whitespace
errors to honor the new configuration.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h       |    9 +++++++++
 config.c      |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 diff.c        |   13 ++++++++-----
 environment.c |    1 +
 4 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index bfffa05..a6e5988 100644
--- a/cache.h
+++ b/cache.h
@@ -602,4 +602,13 @@ extern int diff_auto_refresh_index;
 /* match-trees.c */
 void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
 
+/*
+ * whitespace rules.
+ * used by both diff and apply
+ */
+#define WS_TRAILING_SPACE	01
+#define WS_SPACE_BEFORE_TAB	02
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
+extern unsigned whitespace_rule;
+
 #endif /* CACHE_H */
diff --git a/config.c b/config.c
index dc3148d..ffb418c 100644
--- a/config.c
+++ b/config.c
@@ -246,6 +246,53 @@ static unsigned long get_unit_factor(const char *end)
 	die("unknown unit: '%s'", end);
 }
 
+static struct whitespace_rule {
+	const char *rule_name;
+	unsigned rule_bits;
+} whitespace_rule_names[] = {
+	{ "trailing-space", WS_TRAILING_SPACE },
+	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
+};
+
+static unsigned parse_whitespace_rule(const char *string)
+{
+	unsigned rule = WS_DEFAULT_RULE;
+
+	while (string) {
+		int i;
+		size_t len;
+		const char *ep;
+		int negated = 0;
+
+		string = string + strspn(string, ", \t\n\r");
+		ep = strchr(string, ',');
+		if (!ep)
+			len = strlen(string);
+		else
+			len = ep - string;
+
+		if (*string == '-') {
+			negated = 1;
+			string++;
+			len--;
+		}
+		if (!len)
+			break;
+		for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) {
+			if (strncmp(whitespace_rule_names[i].rule_name,
+				    string, len))
+				continue;
+			if (negated)
+				rule &= ~whitespace_rule_names[i].rule_bits;
+			else
+				rule |= whitespace_rule_names[i].rule_bits;
+			break;
+		}
+		string = ep;
+	}
+	return rule;
+}
+
 int git_parse_long(const char *value, long *ret)
 {
 	if (value && *value) {
@@ -431,6 +478,11 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.whitespace")) {
+		whitespace_rule = parse_whitespace_rule(value);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/diff.c b/diff.c
index a6aaaf7..2112353 100644
--- a/diff.c
+++ b/diff.c
@@ -508,7 +508,8 @@ static void emit_line_with_ws(int nparents,
 	for (i = col0; i < len; i++) {
 		if (line[i] == '\t') {
 			last_tab_in_indent = i;
-			if (0 <= last_space_in_indent)
+			if ((whitespace_rule & WS_SPACE_BEFORE_TAB) &&
+			    0 <= last_space_in_indent)
 				need_highlight_leading_space = 1;
 		}
 		else if (line[i] == ' ')
@@ -540,10 +541,12 @@ static void emit_line_with_ws(int nparents,
 	tail = len - 1;
 	if (line[tail] == '\n' && i < tail)
 		tail--;
-	while (i < tail) {
-		if (!isspace(line[tail]))
-			break;
-		tail--;
+	if (whitespace_rule & WS_TRAILING_SPACE) {
+		while (i < tail) {
+			if (!isspace(line[tail]))
+				break;
+			tail--;
+		}
 	}
 	if ((i < tail && line[tail + 1] != '\n')) {
 		/* This has whitespace between tail+1..len */
diff --git a/environment.c b/environment.c
index b5a6c69..624dd96 100644
--- a/environment.c
+++ b/environment.c
@@ -35,6 +35,7 @@ int pager_in_use;
 int pager_use_color = 1;
 char *editor_program;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
+unsigned whitespace_rule = WS_DEFAULT_RULE;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
-- 
1.5.3.5.1452.ga93d

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Jakub Narebski @ 2007-11-02  7:28 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <200711020825.23464.jnareb@gmail.com>

Jakub Narebski wrote:
> Petr Baudis wrote:
> > On Fri, Nov 02, 2007 at 01:04:03AM +0100, Jakub Narebski wrote:
> 
> >> Is 'getopts' bash-ism, or is it in POSIX?
> > 
> > 	http://www.opengroup.org/onlinepubs/009695399/utilities/getopts.html
> > 
> > (Also, most modern distributions have manpage section 1p (3p, ...) with
> > the same contents, so you can check for this stuff pretty easily.)
> 
> Thanks.
> 
> I have just realized however that it doesn't help any (option parser
> not only for C builtin, but also for shell scripts, Perl scripts and
> Python scripts). First, we (the git development community) do not
> consider fact that something is in POSIX as indicator that we can use
> the construct. Second, getopts delas IIRC only with _short_ options.

Just a thought:
  http://www.systhread.net/texts/200704optparse.php

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Jakub Narebski @ 2007-11-02  7:25 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20071102022335.GU18279@machine.or.cz>

Petr Baudis napisał:
> On Fri, Nov 02, 2007 at 01:04:03AM +0100, Jakub Narebski wrote:

>> Is 'getopts' bash-ism, or is it in POSIX?
> 
> 	http://www.opengroup.org/onlinepubs/009695399/utilities/getopts.html
> 
> (Also, most modern distributions have manpage section 1p (3p, ...) with
> the same contents, so you can check for this stuff pretty easily.)

Thanks.

I have just realized however that it doesn't help any (option parser
not only for C builtin, but also for shell scripts, Perl scripts and
Python scripts). First, we (the git development community) do not
consider fact that something is in POSIX as indicator that we can use
the construct. Second, getopts delas IIRC only with _short_ options.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Steffen Prohaska @ 2007-11-02  7:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, git
In-Reply-To: <7vfxzpbtxv.fsf@gitster.siamese.dyndns.org>


On Nov 1, 2007, at 9:18 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
>>
>>> Steffen Prohaska wrote:
>>>
>>>> You're forced to do the integration immediately.
>
> The context of this "forced" is that you say (in the following
> paragraph) the user's main objective was to "push", but I do not
> think "to push" is ever the main objective.

Right. I should probably describe a bit more of the context.

We have a shared branch for a group of developer who are located
in the same building. We are allowed to commit reasonably stable
code to this branch. Changes should compile and the commiter
should be convinced that it does something useful without
breaking other code. But failing to meet these requirements
is acceptable. For example it is sufficient to compile on
only one architecture and have good reason to believe that
the other architectures will work, too.

A nightly job builds the shared branch on all of our
architectures and creates a report that is available the next
day. If problems happen you should fix them asap. If someone
spots problems causes by others that need to be addressed
right away, he can walk over to the office of the one who
caused the problem.

In this setting a user really want to push. Because only then
the code will be tested and available for all others. ...



>  - If it is to give integrated result for others to work further
>    on, then you need to resolve before being able to achieve
>    that goal.  There is no escaping from it.
>
>  - On the other hand, if it is to show what you did as early as
>    possible in a working shape, and if the updated shared
>    repository has changes from somebody else that conflicts you,
>    in a CVS/SVN style shared workflow, there is no way for you
>    to show what you did in isolation.  If you try to follow that
>    model in git and insist pushing to the same branch, then you
>    are forced to resolve first.
>
>    But you do not have to.  You could push out to another new
>    branch, and say "Here is how you could do it, although this
>    is based on an older codebase and conflicts with what
>    recently happened to the tip".  You could even ask other
>    party whose changes conflict with yours to help with the
>    merge by saying "I pushed it out, you are more familiar with
>    that area of the code and with your changes near the tip of
>    the trunk, so could you merge it and push out the result?"


... I know we could use git to establish a more complex workflow
that would give better guarantees on the published branches.

But it's a judgement how much complexity you want to
add. Pushing to a different branch instead of solving
conflicts right away may be a good model to postpone conflict
resolution. But it requires more knowledge of git and more
commands. Right now, the users are trained on a CVS workflow
and they expect that conflicts may occur and if so need to
be addressed right away. The next step is probably to learn
how git could help them to do this.  (index vs. work tree,
mergetool, ...)

Btw, I have another 'stable' branch, which I have full control
over. This branch is built and tested prior to pushing to the
public repository. So, if the shared branch completely breaks
down, we can fall-back to the stable branch.

We haven't figured out much more of our workflow. The first
milestone is to migrate from CVS to git continuing to use a
CVS-style workflow.



>>> Yes, but you get to choose how. Perhaps git-push should list more
>>> options than just git-pull, such as the three commands required to
>>> rebase the currently checked out branch onto its remote counterpart.
>>> That would support more workflows.
>>
>> I agree. Providing better hints would be good.
>
> I am not so sure about that.  If there are three different
> workflows, should git-push give hints suitable for all of them?
>
> The current hint was added in response to users' requests, and I
> think it could be generalized.  What we would want the end user
> to realize is:
>
>     What I tried to push out is stale, I do not want to push out
>     something that does not contain what the other side has
>     done, so I need to integrate my work with what the other
>     side have before pushing to that branch at the remote.
>
>     In my workflow, that means doing rebase of the branch I
>     tried to push out on top of the remote branch I was trying
>     to push to.
>
> The second paragraph depends on the workflow.  Do we want to
> (can we afford the space to) give a laundry list here?  Probably
> not.

I agree.

But how many different ways of integrating do we have? I only know
of merge or rebase. So, we may just mention both.

Or we only print an extended message if '--verbose' is given. The
short message could be even shorter and refer to '--verbose':

error: remote 'refs/heads/master' is ahead of local 'refs/heads/ 
master'. Use --verbose for more details.

If the user passes --verbose he gets the full story:

- A more detailed description of 'ahead'. For example,
   local could be a strict subset of remote, or local could have
   new commits that are not already at remote.

- We could give all sorts of hints, for example how to list the
   commits that are new on the local side. Recommendations how to
   solve the issue (merge, rebase). The message shouldn't get
   too verbose, though.



>>>> Your main objective was to push, but the shared workflow forces
>>>> you to do the integration _now_ (by using pull). In a pull-only
>>>> workflow, you can just push and defer the integration for later.
>>>
>>> No, you can also fetch + rebase.
>>
>> Right. My point was than one cannot defer the integration. It
>> must be addressed immediately.
>
> See above.

See above.


>>>> Some people claim fetch + rebase is superior to fetch + merge.
>>>> The only point I can see is that fetch + rebase gives a linear
>>>> history without loops, which is nicer to visualize. I recently
>>>> asked on the list if there are any benefits of fetch + rebase
>>>> over fetch + merge, besides a nicer visualization.
>>>
>>>
>>> It's easier to bisect...
>>> With a mostly linear history, this problem goes away.
>>
>> This is really an interesting point. I did not start to use
>> git bisect regularly. But I certainly plan to do so in the future.
>>
>> Couldn't bisect learn to better cope with non-linear history?
>
> It copes with it as best as it can.

I should try out git bisect to understand the details.


> Another thing to think about is how "everybody fetches, merges
> and pushes out" would interact with the concept of "mainline".
> Strictly speaking, the point of distributed development is that
> there is no mainline, but workflows based on "fetch + rebase"
> allows --first-parent to give a reasonable approximation of what
> people would naively expect how the mainline would look like.
> If everybody fetches, merges and pushes out, there is no
> "mainline" and --first-parent would give totally useless
> history.

Building a main line needs more control and more knowledge
about git.

Here is what I think can be done. It's only a sketch so
far. It's not yet reality. Therefore it might turn out to be
infeasible. I'd adjust my plans then.

We actually have at least three groups of developers that work
at three different locations. They'll work on different shared
branches. We also will create shared topic branches if a smaller
group of developers needs to work together on a prototype.

At some point shared branches need to become stable. They
need to be tested and maybe some of the changes need to be
reverted if they turn out to be useless. Finally we'll have a
tip of a shared branch that is stable. Stable depends on the
quality criteria, which may vary depending on where we are
in the release cycle. But at least some minimal requirements,
like "compiles on all platforms" or "passes all tests" will be
verified. Such a stable tip will now be merged with '--no-ff'
to the mainline. The merge will be thoroughly tested.

The chain of commits along first parent establishes a mainline
that matches certain quality criteria. The criteria are not
necessarily met by commits on the side branches. Therefore
fast-forward must not be used for the merge.

If we feel comfortable with git, we may consider creating
better topic branches in the first place. But for now I want
to start with shared branches containing a mixed bag of
commits.

Do all this make sense?

	Steffen

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Miles Bader @ 2007-11-02  6:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999.0711011129460.3342@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:
> So while you can combine flags for *most* programs, you still won't 
> be able to say things like
>
> 	git clean -qdx
>
> just because that's still a shellscript, and doing any fancy argument 
> parsing in shell is just painful.

Indeed... but for my personal shell scripts I like to use a construct
like the following for parsing args:

   while :; do
     case "$1" in
        ... lots of cases to handle normal args ...

       -[!-]?*)
         # split concatenated single-letter options apart
         FIRST="$1"; shift
         set -- `echo $FIRST | $SED 's/-\(.\)\(.*\)/-\1 -\2/'` "$@"
         ;;

       -*)
         # unrecognized option
         unrec_opt "$1"; exit 1;;
       *)
         # non-option
         break;
     esac
   done

I'm sure there are problems with it, but it generally seems to work
pretty reasonably for short options.

-Miles
-- 
97% of everything is grunge

^ permalink raw reply

* Re: [PATCH] Fixed a gcc 4.0.1 complaint about an uninitialized variable
From: Junio C Hamano @ 2007-11-02  5:37 UTC (permalink / raw)
  To: Blake Ramsdell; +Cc: git
In-Reply-To: <1193971102-61907-2-git-send-email-blaker@gmail.com>

Blake Ramsdell <blaker@gmail.com> writes:

> diff --git a/transport.c b/transport.c
> index 400af71..cac1870 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -107,7 +107,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
>  		return;
>  
>  	for (;;) {
> -		int cmp, len;
> +		int cmp = 0, len;

Yeah, if you follow the logic, it is clear that the variable is
never used while unset, but gcc is not careful enough to see it.

It is customary to use

	int cmp = cmp;

for something like this.  There are already other instances of
such phony initializations in the code elsewhere.

^ permalink raw reply

* Re: Debugging corrupt object generation
From: Shawn O. Pearce @ 2007-11-02  5:15 UTC (permalink / raw)
  To: Julian Phillips; +Cc: Nicolas Pitre, git
In-Reply-To: <Pine.LNX.4.64.0711020018400.18429@beast.quantumfyre.co.uk>

Julian Phillips <julian@quantumfyre.co.uk> wrote:
> On Thu, 1 Nov 2007, Nicolas Pitre wrote:
> >
> >Maybe fast-import hasn't flushed the needed data to the pack yet?
> 
> Well, fast-import completes quite happily and outputs the normal summary 
> status.  I can look at logs and trees etc provided that I don't try and 
> look at one particular part of the tree on one particular commit.  I 
> think the problem is that I've managed to do something inside fast-import 
> that corrupts one particular tree object (though I've no idea what that 
> might be).

Ahhh.  I'm betting you messed up the version 0 and version 1 arrays
inside of the struct tree_entry.  This could cause the delta
generator to look at the wrong base information when it creates
the delta, thus causing the delta to be created for a different
base than what the object is actually using in the packfile.

The version[0] is meant to hold the mode and SHA-1 of the tree_entry
in the base object.  The version[1] is meant to hold the current
mode and SHA-1 of the tree_entry in the new object.  A mode of 0
means the entry doesn't exist in that particular tree; so an add
is shown as "version[0].mode = 0; version[1].mode = 0100644".

Look at the store_tree function, this is where we regenerate the
canonical representation of both the version[0] and the version[1]
trees so the store_object function can generate a delta.  Note that
we assume the base object name is root->versions[0].sha1.  Maybe the
version[0] array doesn't actually match the tree named by that sha1?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/3] Introduce --dirty option to git-rebase, allowing you to start from a dirty state.
From: Shawn O. Pearce @ 2007-11-02  5:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Simon Sasburg, git
In-Reply-To: <7vir4l8ug4.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Doesn't this have the exact same problem with the one in 'next'
> > that uses "git-stash create", which Shawn said he was upset
> > about, and I said I will revert?
> 
> Sorry, --dirty is not the default, which changes everything.
> Forget what I said, sorry for the noise.

I'm happy with having --dirty, but AS AN OPTION.  Heck, I'd probably
use it sometimes, but only if it also reapplies the stash after
the rebase is complete.  But doing that stash/apply automatically
is really freaking annoying.

For the same reasons why I like git-checkout not defaulting to -m.
I want Git to stop by default if I'm about to possibly go into a
command that is going to cause conflicts, as I may not be ready to
deal with them right now.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 3/3] Show total transferred as part of throughput progress
From: Shawn O. Pearce @ 2007-11-02  4:59 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <1193950797-29631-4-git-send-email-nico@cam.org>

Nicolas Pitre <nico@cam.org> wrote:
> Right now it is infeasible to offer to the user a reasonable concept
> of when a clone will be complete as we aren't able to come up with
> the final pack size until after we have actually transferred the
> entire thing to the client.  However in many cases users can work
> with a rough rule-of-thumb; for example it is somewhat well known
> that git.git is about 16 MiB today and that linux-2.6.git is over
> 120 MiB.
...
> [from an initial proposal from Shawn O. Pearce]

Thanks for rewriting this.  I agree, your replacement patch is
much better looking than my proposal.  I also see Junio has already
applied it to next.  Excellent.
 
-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] make the pack index version configurable
From: Nicolas Pitre @ 2007-11-02  4:01 UTC (permalink / raw)
  To: David Symonds; +Cc: Junio C Hamano, git
In-Reply-To: <ee77f5c20711012042r39c6303em2a8140e8348051d@mail.gmail.com>

On Fri, 2 Nov 2007, David Symonds wrote:

> On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
> >
> > +pack.indexVersion::
> > +       Specify the default pack index version.  Valid values are 1 for
> > +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> > +       the new pack index with capabilities for packs larger than 4 GB
> > +       as well as proper protection against the repacking of corrupted
> > +       packs.  Version 2 is selected and this config option ignored
> > +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> > +       the default is 1.
> 
> If you're trying to force a previous pack version for some reason
> (backward compatibility, or whatever), this "feature" of automatic
> forcing version 2 for 2 GB packs might come as a nasty suprise.
> Wouldn't it be better to fail with an error?

No.  The fact is that you don't know in advance what the pack size will 
be, and reaching that limit might take a long while already if 
repacking, or even more so if the pack is fetched over a network 
connection.  That would be wasteful to simply throw away all the whole 
repack/download with an error which would be an even nastier surprise at 
that point just because you specified index v1.

And such a pack would be impossible to work with using an old Git 
version anyway, so the old Git version will happily give you the error, 
suggesting that you upgrade to a later version.

And this has been the implemented behavior for the last 7 months 
already.

And the idea is to switch the default to version 2 eventually.

Etc.


Nicolas

^ permalink raw reply

* [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set
From: Blake Ramsdell @ 2007-11-02  2:38 UTC (permalink / raw)
  To: gitster, git; +Cc: Blake Ramsdell

Signed-off-by: Blake Ramsdell <blaker@gmail.com>
---
 Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 71479a2..5d83756 100644
--- a/Makefile
+++ b/Makefile
@@ -401,7 +401,9 @@ endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
-	OLD_ICONV = UnfortunatelyYes
+	ifneq ($(uname_R),9.0.0)
+		OLD_ICONV = UnfortunatelyYes
+	endif
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
 endif
-- 
1.5.3.GIT

^ permalink raw reply related

* [PATCH] Fixed a gcc 4.0.1 complaint about an uninitialized variable
From: Blake Ramsdell @ 2007-11-02  2:38 UTC (permalink / raw)
  To: gitster, git; +Cc: Blake Ramsdell
In-Reply-To: <1193971102-61907-1-git-send-email-blaker@gmail.com>

Signed-off-by: Blake Ramsdell <blaker@gmail.com>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index 400af71..cac1870 100644
--- a/transport.c
+++ b/transport.c
@@ -107,7 +107,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 		return;
 
 	for (;;) {
-		int cmp, len;
+		int cmp = 0, len;
 
 		if (!fgets(buffer, sizeof(buffer), f)) {
 			fclose(f);
-- 
1.5.3.GIT

^ permalink raw reply related

* [PATCH] pack-objects: get rid of an ugly cast
From: Nicolas Pitre @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

... when calling write_idx_file().

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 25ec65d..a9d7633 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -57,7 +57,7 @@ struct object_entry {
  * nice "minimum seek" order.
  */
 static struct object_entry *objects;
-static struct object_entry **written_list;
+static struct pack_idx_entry **written_list;
 static uint32_t nr_objects, nr_alloc, nr_result, nr_written;
 
 static int non_empty;
@@ -577,7 +577,7 @@ static off_t write_one(struct sha1file *f,
 		e->idx.offset = 0;
 		return 0;
 	}
-	written_list[nr_written++] = e;
+	written_list[nr_written++] = &e->idx;
 
 	/* make sure off_t is sufficiently large not to wrap */
 	if (offset > offset + size)
@@ -599,7 +599,7 @@ static void write_pack_file(void)
 
 	if (do_progress)
 		progress_state = start_progress("Writing objects", nr_result);
-	written_list = xmalloc(nr_objects * sizeof(struct object_entry *));
+	written_list = xmalloc(nr_objects * sizeof(*written_list));
 
 	do {
 		unsigned char sha1[20];
@@ -651,9 +651,8 @@ static void write_pack_file(void)
 			umask(mode);
 			mode = 0444 & ~mode;
 
-			idx_tmp_name = write_idx_file(NULL,
-					(struct pack_idx_entry **) written_list,
-					nr_written, sha1);
+			idx_tmp_name = write_idx_file(NULL, written_list,
+						      nr_written, sha1);
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
 			if (adjust_perm(pack_tmp_name, mode))
@@ -677,7 +676,7 @@ static void write_pack_file(void)
 
 		/* mark written objects as written to previous pack */
 		for (j = 0; j < nr_written; j++) {
-			written_list[j]->idx.offset = (off_t)-1;
+			written_list[j]->offset = (off_t)-1;
 		}
 		nr_remaining -= nr_written;
 	} while (nr_remaining && i < nr_objects);

^ permalink raw reply related

* [PATCH] git-p4: Detect changes to executable bit and include them in p4 submit.
From: Chris Pettitt @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Simon Hausmann, git; +Cc: Chris Pettitt
In-Reply-To: <1193974994-19211-1-git-send-email-cpettitt@gmail.com>

This changeset takes advantage of the new parseDiffTreeEntry(...) function to
detect changes to the execute bit in the git repository.  During submit, git-p4
now looks for changes to the executable bit and if it finds them it "reopens"
the file in perforce, which allows it to change the file type.

The logic for adding the executable bit in perforce is straightforward: the +x
modifier can be used. Removing the executable bit in perforce requires that the
entire filetype be redefined (there is no way to join remove the bit with a -x
modifier, for example). This changeset includes logic to remove the executable
bit from the full file type while preserving the base file type and other
modifiers.

Signed-off-by: Chris Pettitt <cpettitt@gmail.com>
---
 contrib/fast-import/git-p4 |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index c7fc564..c148b5a 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -71,6 +71,31 @@ def isP4Exec(kind):
     a plus sign, it is also executable"""
     return (re.search(r"(^[cku]?x)|\+.*x", kind) != None)
 
+def setP4ExecBit(file, mode):
+    # Reopens an already open file and changes the execute bit to match
+    # the execute bit setting in the passed in mode.
+
+    p4Type = "+x"
+
+    if not isModeExec(mode):
+        p4Type = getP4OpenedType(file)
+        p4Type = re.sub('^([cku]?)x(.*)', '\\1\\2', p4Type)
+        p4Type = re.sub('(.*?\+.*?)x(.*?)', '\\1\\2', p4Type)
+        if p4Type[-1] == "+":
+            p4Type = p4Type[0:-1]
+
+    system("p4 reopen -t %s %s" % (p4Type, file))
+
+def getP4OpenedType(file):
+    # Returns the perforce file type for the given file.
+
+    result = read_pipe("p4 opened %s" % file)
+    match = re.match(".*\((.+)\)$", result)
+    if match:
+        return match.group(1)
+    else:
+        die("Could not determine file type for %s" % file)
+
 def diffTreePattern():
     # This is a simple generator for the diff tree regex pattern. This could be
     # a class variable if this and parseDiffTreeEntry were a part of a class.
@@ -111,6 +136,14 @@ def parseDiffTreeEntry(entry):
         }
     return None
 
+def isModeExec(mode):
+    # Returns True if the given git mode represents an executable file,
+    # otherwise False.
+    return mode[-3:] == "755"
+
+def isModeExecChanged(src_mode, dst_mode):
+    return isModeExec(src_mode) != isModeExec(dst_mode)
+
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
@@ -538,15 +571,19 @@ class P4Submit(Command):
         filesToAdd = set()
         filesToDelete = set()
         editedFiles = set()
+        filesToChangeExecBit = {}
         for line in diff:
             diff = parseDiffTreeEntry(line)
             modifier = diff['status']
             path = diff['src']
             if modifier == "M":
                 system("p4 edit \"%s\"" % path)
+                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
+                    filesToChangeExecBit[path] = diff['dst_mode']
                 editedFiles.add(path)
             elif modifier == "A":
                 filesToAdd.add(path)
+                filesToChangeExecBit[path] = diff['dst_mode']
                 if path in filesToDelete:
                     filesToDelete.remove(path)
             elif modifier == "D":
@@ -557,6 +594,8 @@ class P4Submit(Command):
                 src, dest = diff['src'], diff['dst']
                 system("p4 integrate -Dt \"%s\" \"%s\"" % (src, dest))
                 system("p4 edit \"%s\"" % (dest))
+                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
+                    filesToChangeExecBit[dest] = diff['dst_mode']
                 os.unlink(dest)
                 editedFiles.add(dest)
                 filesToDelete.add(src)
@@ -609,6 +648,11 @@ class P4Submit(Command):
             system("p4 revert \"%s\"" % f)
             system("p4 delete \"%s\"" % f)
 
+        # Set/clear executable bits
+        for f in filesToChangeExecBit.keys():
+            mode = filesToChangeExecBit[f]
+            setP4ExecBit(f, mode)
+
         logMessage = ""
         if not self.directSubmit:
             logMessage = extractLogMessageFromGitCommit(id)
-- 
1.5.3.4.498.g9c514

^ permalink raw reply related

* [PATCH] git-p4: Add a helper function to parse the full git diff-tree output.
From: Chris Pettitt @ 2007-11-02  3:43 UTC (permalink / raw)
  To: Simon Hausmann, git; +Cc: Chris Pettitt


Signed-off-by: Chris Pettitt <cpettitt@gmail.com>
---
 contrib/fast-import/git-p4 |   49 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index bf33f74..c7fc564 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -71,6 +71,46 @@ def isP4Exec(kind):
     a plus sign, it is also executable"""
     return (re.search(r"(^[cku]?x)|\+.*x", kind) != None)
 
+def diffTreePattern():
+    # This is a simple generator for the diff tree regex pattern. This could be
+    # a class variable if this and parseDiffTreeEntry were a part of a class.
+    pattern = re.compile(':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
+    while True:
+        yield pattern
+
+def parseDiffTreeEntry(entry):
+    """Parses a single diff tree entry into its component elements.
+
+    See git-diff-tree(1) manpage for details about the format of the diff
+    output. This method returns a dictionary with the following elements:
+
+    src_mode - The mode of the source file
+    dst_mode - The mode of the destination file
+    src_sha1 - The sha1 for the source file
+    dst_sha1 - The sha1 fr the destination file
+    status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc)
+    status_score - The score for the status (applicable for 'C' and 'R'
+                   statuses). This is None if there is no score.
+    src - The path for the source file.
+    dst - The path for the destination file. This is only present for
+          copy or renames. If it is not present, this is None.
+
+    If the pattern is not matched, None is returned."""
+
+    match = diffTreePattern().next().match(entry)
+    if match:
+        return {
+            'src_mode': match.group(1),
+            'dst_mode': match.group(2),
+            'src_sha1': match.group(3),
+            'dst_sha1': match.group(4),
+            'status': match.group(5),
+            'status_score': match.group(6),
+            'src': match.group(7),
+            'dst': match.group(10)
+        }
+    return None
+
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b'):
     cmd = "p4 -G %s" % cmd
     if verbose:
@@ -494,13 +534,14 @@ class P4Submit(Command):
         else:
             print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
             diffOpts = ("", "-M")[self.detectRename]
-            diff = read_pipe_lines("git diff-tree -r --name-status %s \"%s^\" \"%s\"" % (diffOpts, id, id))
+            diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
         filesToAdd = set()
         filesToDelete = set()
         editedFiles = set()
         for line in diff:
-            modifier = line[0]
-            path = line[1:].strip()
+            diff = parseDiffTreeEntry(line)
+            modifier = diff['status']
+            path = diff['src']
             if modifier == "M":
                 system("p4 edit \"%s\"" % path)
                 editedFiles.add(path)
@@ -513,7 +554,7 @@ class P4Submit(Command):
                 if path in filesToAdd:
                     filesToAdd.remove(path)
             elif modifier == "R":
-                src, dest = line.strip().split("\t")[1:3]
+                src, dest = diff['src'], diff['dst']
                 system("p4 integrate -Dt \"%s\" \"%s\"" % (src, dest))
                 system("p4 edit \"%s\"" % (dest))
                 os.unlink(dest)
-- 
1.5.3.4.498.g9c514

^ permalink raw reply related

* Re: [PATCH] make the pack index version configurable
From: David Symonds @ 2007-11-02  3:42 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.9999.0711012312480.21255@xanadu.home>

On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
>
> +pack.indexVersion::
> +       Specify the default pack index version.  Valid values are 1 for
> +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> +       the new pack index with capabilities for packs larger than 4 GB
> +       as well as proper protection against the repacking of corrupted
> +       packs.  Version 2 is selected and this config option ignored
> +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> +       the default is 1.

If you're trying to force a previous pack version for some reason
(backward compatibility, or whatever), this "feature" of automatic
forcing version 2 for 2 GB packs might come as a nasty suprise.
Wouldn't it be better to fail with an error?


Dave.

^ permalink raw reply

* git-p4: detect changes to executable bit and push to p4
From: Chris Pettitt @ 2007-11-02  3:41 UTC (permalink / raw)
  To: Simon Hausmann, git


The following two patches add support to git-p4 for detecting changes to the
executable bit in git and including these changes in the p4 changeset during
git-p4 submit.

^ permalink raw reply

* Re: [PATCH] post-update hook: update working copy
From: Sam Vilain @ 2007-11-02  3:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sam Vilain, git
In-Reply-To: <7vd4ut7948.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
>> Now that git-stash is available, it is not so unsafe to push to a
>> non-bare repository, but care needs to be taken to preserve any dirty
>> working copy or index state.  This hook script does that, using
>> git-stash.
> 
> Honestly, I am reluctant to do things that _encourages_ pushing
> into a live tree.

I think a lot of people want this, and explaining why it doesn't work to
people is especially tiresome given that I know it can work, and the
caveats get smaller and smaller each time.  I still think that the
operation can be made safe enough for general use.

>  - Who guarantees that the reflog is enabled for the HEAD?

Ok, so I guess if that's the case then the old behaviour is OK.
However, this would not be required if implemented in receive-pack, as
the only thing I'm using it for is to get the revision that the current
index is based on.

The other option, which was writing two hooks, both of which need to be
enabled, seemed far too ugly!

>  - Who guarantees that a human user is not actively editing the
>    work tree files without saving?  You would not see "dirty
>    state", the editor would notice "the file was modified since
>    you started editing it" and tell so to the user, but the user
>    cannot recover from the situation without knowing to do the
>    three-way merge between HEAD@{1}, HEAD and the index _anyway_.

There seems to be a lot of focus on this.  However I don't think it is a
showstopper; in this instance that the person who has their life ruined
by the incoming push can blame the person who did it, blame themselves
for giving that stupid user access to their working directory, and then
accept that the tool is doing the best that it can.

>> +	if git diff-index HEAD@{1} >/dev/null
> 
> Are you missing "--cached" here?

Ah, yes, good catch.

>> +	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
>> +	then
>> +		new=$(git rev-parse HEAD)
>> +		git-update-ref --no-deref HEAD HEAD@{1}
>> +		echo "W:stashing dirty $desc - see git-stash(1)" >&2
>> +		(cd $GIT_WORK_TREE
>> +		git stash save "dirty $desc before update to $new")
>> +		git-symbolic-ref HEAD "$ref"
> 
> This part feels somewhat dangerous.  What happens if we are
> interrupted in the middle of these commands?

Heh.  What seems to happen is that the local command is aborted and the
remote one continues.  Nonetheless I'll add a trap statement, but again
if implemented in receive-pack it could probably be more resilient.

>> +	fi
>> +
>> +	# eye candy - show the WC updates :)
>> +	echo "Updating working copy" >&2
>> +	(cd $GIT_WORK_TREE
>> +	git-diff-index -R --name-status HEAD >&2
>> +	git-reset --hard HEAD)
>> +}
> 
> And I would have expected you would unstash the dirty state here.
> Are there any reason not to?

If git-stash could support stashing conflicted merges, I don't think so.

However, if the user is a git-shell user, then they won't be able to
resolve the conflict and they won't be able to re-push as the stash will
fail (a condition not visited by the above).

Sam.

changes as you suggested are below;

diff --git a/templates/hooks--post-update b/templates/hooks--post-update
index 352a432..b15c711 100755
--- a/templates/hooks--post-update
+++ b/templates/hooks--post-update
@@ -25,6 +25,11 @@ fi
 update_wc() {
 	ref=$1
 	echo "Push to checked out branch $ref" >&2
+	if [ ! -f $GIT_DIR/logs/HEAD ]
+	then
+		echo "E:push to non-bare repository requires a HEAD reflog" >&2
+		exit 1
+	fi
 	if (cd $GIT_WORK_TREE; git-diff-files -q --exit-code >/dev/null)
 	then
 		wc_dirty=0
@@ -33,7 +38,7 @@ update_wc() {
 		wc_dirty=1
 		desc="working copy"
 	fi
-	if git diff-index HEAD@{1} >/dev/null
+	if git diff-index --cached HEAD@{1} >/dev/null
 	then
 		index_dirty=0
 	else
@@ -49,11 +54,13 @@ update_wc() {
 	if [ "$wc_dirty" -ne 0 -o "$index_dirty" -ne 0 ]
 	then
 		new=$(git rev-parse HEAD)
-		git-update-ref --no-deref HEAD HEAD@{1}
 		echo "W:stashing dirty $desc - see git-stash(1)" >&2
-		(cd $GIT_WORK_TREE
-		git stash save "dirty $desc before update to $new")
+		( trap 'echo trapped $$; git symbolic-ref HEAD "'"$ref"'"' 2 3 13 15 ERR EXIT
+		git-update-ref --no-deref HEAD HEAD@{1}
+		cd $GIT_WORK_TREE
+		git stash save "dirty $desc before update to $new";
 		git-symbolic-ref HEAD "$ref"
+		)
 	fi
 
 	# eye candy - show the WC updates :)

^ permalink raw reply related

* [PATCH] make the pack index version configurable
From: Nicolas Pitre @ 2007-11-02  3:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

It is a good idea to use pack index version 2 all the time since it has
proper protection against propagation of certain pack corruptions when
repacking which is not possible with index version 1, as demonstrated
in test t5302.

Hence this config option.

The default is still pack index version 1.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d4a476e..862b79e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -660,6 +660,15 @@ pack.threads::
 	machines. The required amount of memory for the delta search window
 	is however multiplied by the number of threads.
 
+pack.indexVersion::
+	Specify the default pack index version.  Valid values are 1 for
+	legacy pack index used by Git versions prior to 1.5.2, and 2 for
+	the new pack index with capabilities for packs larger than 4 GB
+	as well as proper protection against the repacking of corrupted
+	packs.  Version 2 is selected and this config option ignored
+	whenever the corresponding pack is larger than 2 GB.  Otherwise
+	the default is 1.
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 25ec65d..e923689 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1768,6 +1768,12 @@ static int git_pack_config(const char *k, const char *v)
 #endif
 		return 0;
 	}
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
 	return git_default_config(k, v);
 }
 
diff --git a/index-pack.c b/index-pack.c
index 61ea762..715a5bb 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -683,6 +683,17 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 	}
 }
 
+static int git_index_pack_config(const char *k, const char *v)
+{
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
+	return git_default_config(k, v);
+}
+
 int main(int argc, char **argv)
 {
 	int i, fix_thin_pack = 0;
@@ -693,6 +704,8 @@ int main(int argc, char **argv)
 	struct pack_idx_entry **idx_objects;
 	unsigned char sha1[20];
 
+	git_config(git_index_pack_config);
+
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Petr Baudis @ 2007-11-02  2:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <fgdphj$6ga$1@ger.gmane.org>

On Fri, Nov 02, 2007 at 01:04:03AM +0100, Jakub Narebski wrote:
> Is 'getopts' bash-ism, or is it in POSIX?

	http://www.opengroup.org/onlinepubs/009695399/utilities/getopts.html

(Also, most modern distributions have manpage section 1p (3p, ...) with
the same contents, so you can check for this stuff pretty easily.)

-- 
				Petr "Pasky" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Petr Baudis @ 2007-11-02  2:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Johan Herland, git, Geert Bosch
In-Reply-To: <alpine.LFD.0.999.0711011459490.3342@woody.linux-foundation.org>

On Thu, Nov 01, 2007 at 03:05:44PM -0700, Linus Torvalds wrote:
> On Thu, 1 Nov 2007, Junio C Hamano wrote:
> > 
> > "git clean" is about things that git usually do not care about
> > (i.e. things not in .gitignore, or even in .gitignore when -x is
> > given).  Everything else including "git stash" is all about what
> > git cares about (i.e. tracked paths).

BTW, it comes from Cogito. Pavel Roskin is the author of the original
Cogito script and I'm not sure if it came there from anywhere else or is
an original "invention".

> Right. I *love* "git clean". Real men have everything they care about 
> tracked by git, and thus by definition "git clean" is the safest operation 
> possible. I don't understand how anybody can call it "unsafe".

I agree! If you want to keep anything around in git-tracked tree, tell
git about it! (Either marking it as ignored or adding it to the index.)
I think I wasn't too fond of it originally but now tend to use it a lot
to keep my trees clean of temporary cruft.

> I just wish it was quiet by default - right now it takes a _loong_ time to 
> clean out your crud just because it scrolls forever talking about all 
> those girly files you don't want to keep - and that it had -x and -d on by 
> default, so that us *real* men wouldn't have to type so much.

I do not agree with either, though. Having it verbose by default makes
it at least obvious that something potentially, uh, surprising is going
on; and I _prefer_ the non-x behaviour. If I told git that it should
ignore $file, it better not touch it.

> But yeah, I guess we could make the "clean.Imagirlyman" option (or however 
> the "please-don't-hurt-me-by-default" option is spelled) the default. That 
> way I'd just feel *extra* manly for immediately disabling it, and laughing 
> at you wimps.

Yeah!

-- 
				Petr "Pasky, laughing at the wimps" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan

^ permalink raw reply

* git rm --cached
From: Jing Xue @ 2007-11-02  2:17 UTC (permalink / raw)
  To: git


In the following scenario, why do I have to run 'git reset' following
'git rm --cached 1.txt' to revert to exactly where I was before 'git add
1.txt'?  Shouldn't 'git rm --cached' have done that already?

jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   1.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
jingxue@fawkes:~/workspace/t1.git$ git add 1.txt
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   1.txt
#
jingxue@fawkes:~/workspace/t1.git$ git rm --cached 1.txt
rm '1.txt'
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       deleted:    1.txt
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       1.txt
jingxue@fawkes:~/workspace/t1.git$ git reset
1.txt: needs update
jingxue@fawkes:~/workspace/t1.git$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   1.txt
#
no changes added to commit (use "git add" and/or "git commit -a")

Thanks.
-- 
Jing Xue

^ 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