Git development
 help / color / mirror / Atom feed
* [PATCH] gitk: Add shortcut Ctrl-W for closing the active window
From: Jens Lehmann @ 2010-02-02 22:11 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Git Mailing List

To make the user experience between git gui and gitk more homogeneous, use
Ctrl-W in gitk too for closing the active window. When closing the main
window doquit is called for proper cleanup.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Found myself hitting Ctrl-W in gitk with no effect once too often ;-)

 gitk-git/gitk |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 03dd0f4..9629394 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2383,6 +2383,8 @@ proc makewindow {} {
     }
     bindall <$::BM> "canvscan mark %W %x %y"
     bindall <B$::BM-Motion> "canvscan dragto %W %x %y"
+    bind all <$M1B-Key-w> {destroy [winfo toplevel %W]}
+    bind . <$M1B-Key-w> doquit
     bindkey <Home> selfirstline
     bindkey <End> sellastline
     bind . <Key-Up> "selnextline -1"
@@ -2814,6 +2816,7 @@ proc keys {} {
 [mc "Gitk key bindings:"]

 [mc "<%s-Q>		Quit" $M1T]
+[mc "<%s-W>		Close window" $M1T]
 [mc "<Home>		Move to first commit"]
 [mc "<End>		Move to last commit"]
 [mc "<Up>, p, i	Move up one commit"]
-- 
1.7.0.rc1.173.gc7c5.dirty

^ permalink raw reply related

* Re: master^ is not a local branch -- huh?!?
From: Ron Garret @ 2010-02-02 22:04 UTC (permalink / raw)
  To: git
In-Reply-To: <20100202191942.GB9628@fieldses.org>

In article <20100202191942.GB9628@fieldses.org>,
 "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Feb 01, 2010 at 05:12:42PM -0800, Ron Garret wrote:
> > In article <7vk4uwmp95.fsf@alter.siamese.dyndns.org>,
> >  Junio C Hamano <gitster@pobox.com> wrote:
> > 
> > > Ron Garret <ron1@flownet.com> writes:
> > > 
> > > > The manual specifically contradicts you, so either you are wrong or the 
> > > > manual is wrong.
> > > 
> > > In case you haven't noticed, Pasky is one of the old timers and he knows 
> > > a
> > > thing or two about the git's world model.
> > 
> > My intent was not to diss Pasky, it was just to point out a disconnect 
> > between what he was saying and what the manual says.  It's quite 
> > possible that the manual is wrong or out of date or just misleading.  
> > But it says what it says.
> > 
> > > And I do not see a contradiction in what the manual describes and "a
> > > branch is a named pointer to a commit" (although "named" can probably be
> > > omitted as "unnamed pointer" is not useful at the UI level).
> > 
> > But that's not what the manual says.  The manual says, "When we need to 
> > be precise, we will use the word "branch" to mean a line of 
> > development..."  Those are the first words in the section entitled 
> > "Understanding history: What is a branch?"  It certainly appears to the 
> > untrained eye that that is intended to be the definition of a branch.
> 
> My memory is that I'd seen the word "branch" used for both meanings (a
> linear piece of history, and a ref under ref/heads/), so figured we
> needed terms for both.
> 
> But then I didn't really use that distinction anywhere.  On a quick skim
> the only instance I can see of the first sense is in
> http://kernel.org/pub/software/scm/git-core/docs/user-manual.html#counting-com
> mits-on-a-branch,
> which could probably be reworded.
> 
> It still may be worth acknowledging the confusion; e.g., something like:
> 
> 	In the above diagram, "A", "B", and "master" are all references
> 	to a point in history.  We call all three "branches".
> 
> 	Informally, the word "branch" is sometimes also used to the
> 	entire line of development leading up to one of these points,
> 	or, more generally, to any individual line of development.  But
> 	when speaking about git, a "branch" (or "branch head") will
> 	always be a reference to a point in history, and in particular a
> 	reference which may be advanced to new commits by future
> 	development.
> 
> Eh, I don't know if that's helpful; maybe that section could just be
> deleted.  Or replaced by a more general discusion of the ref/ namespace.

FWIW, I find the above verbiage to to be very clear, much better than 
what is there now.  You might also add that branches are almost exactly 
the same as tags.  The only difference (AFAIK) is that tags get dragged 
along by commits and resets and tags don't.

rg

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Erik Faye-Lund @ 2010-02-02 22:03 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Wincent Colaiuta, Avery Pennarun, Jacob Helwig, git
In-Reply-To: <76c5b8581002021356m52bb1817k9a4a29da0d4b681d@mail.gmail.com>

On Tue, Feb 2, 2010 at 10:56 PM, Eugene Sajine <euguess@gmail.com> wrote:
>>
>> As has already been pointed out, an extremely common workflow for "git
>> commit --amend" is to include additional changes that were accidentally
>> overlooked  in the initial commit, and no changes are made to the commit
>> message.
>>
>> Changing the behavior would break that perfectly reasonable,
>> widely-practised workflow, and _that_ would be 100% not ok.
>
> I'm not against that workflow - I'm not forcing anybody to change the
> commit message. The thing I'm talking about is roughly that it should
> NOT treat the result of command ":q" the same way as the command
> ":wq".
>

Quite a lot of text-editors on prevents a user from saving when the
buffer is unchanged, so I don't think this would actually work. Keep
in mind that vi is not the only editor one can use to edit commit
messages.

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* [PATCH 1/2] gitweb: Add an option to force version match
From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw)
  To: git
In-Reply-To: <1265147814-13284-1-git-send-email-warthog9@eaglescrag.net>

From: John 'Warthog9' Hawley <warthog9@kernel.org>

This adds $git_versions_must_match variable, which if set to true,
checks that we are running on the same version of git that we
shipped with, and if not throw '500 Internal Server Error' error.
What is checked is the version of gitweb (embedded in building
gitweb.cgi), against version of runtime git binary used.

Gitweb can usually run with a mismatched git install.  This is more
here to give an obvious warning as to what's going on vs. silently
failing.

By default this feature is turned on.

Add tests to t9501-gitweb-standalone-http-status.sh that this feature
works correctly (as expected) if turned on, both in match and no match
case.

Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/README                            |    5 ++++
 gitweb/gitweb.perl                       |   32 +++++++++++++++++++++++++++++-
 t/gitweb-lib.sh                          |    1 +
 t/t9501-gitweb-standalone-http-status.sh |   27 +++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/gitweb/README b/gitweb/README
index 6c2c8e1..ec93da6 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -233,6 +233,11 @@ not include variables usually directly set during build):
    If server load exceed this value then return "503 Service Unavaliable" error.
    Server load is taken to be 0 if gitweb cannot determine its value.  Set it to
    undefined value to turn it off.  The default is 300.
+ * $git_versions_must_match
+   If set to true value, gitweb fails with "500 Internal Server Error" error
+   if the version of the gitweb doesn't match version of the git binary.
+   Gitweb can usually run with a mismatched git install.   The default is 1
+   (true).
 
 
 Projects list file format
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d0c3ff2..57771a0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -221,6 +221,9 @@ our %avatar_size = (
 	'double'  => 32
 );
 
+# If it is true, exit if gitweb version and git binary version don't match
+our $git_versions_must_match = 1;
+
 # Used to set the maximum load that we will still respond to gitweb queries.
 # If server load exceed this value then return "503 server busy" error.
 # If gitweb cannot determined server load, it is taken to be 0.
@@ -550,10 +553,10 @@ sub filter_snapshot_fmts {
 }
 
 our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
+our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++";
 if (-e $GITWEB_CONFIG) {
 	do $GITWEB_CONFIG;
 } else {
-	our $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || "++GITWEB_CONFIG_SYSTEM++";
 	do $GITWEB_CONFIG_SYSTEM if -e $GITWEB_CONFIG_SYSTEM;
 }
 
@@ -583,6 +586,33 @@ sub get_loadavg {
 our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
 $number_of_git_cmds++;
 
+# Throw an error if git versions does not match, if $git_versions_must_match is true.
+if ($git_versions_must_match &&
+    $git_version ne $version) {
+	my $admin_contact =
+		defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : '';
+	my $err_msg = <<EOT;
+<h1 align="center">*** Warning ***</h1>
+<p>
+This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>,
+however git version <b>@{[esc_html($git_version)]}</b> was found on server.
+Running an instance of gitweb that is not matched to the git binaries may
+result in unexpected behavior of gitweb, and loss of functionality or
+incorrect data on displayed pages.
+</p>
+<p>
+Please update the git or gitweb installation so that their versions match, or
+if you feel you are sure that you wish to proceed with running gitweb
+with unmatched versions please contact the server administrator${admin_contact}
+to configure gitweb to allow mismatched versions.  This can be done by
+setting \$git_versions_must_match to @{[esc_html($git_versions_must_match)]}
+(false value) in gitweb configuration file,
+'@{[esc_path(-e $GITWEB_CONFIG ? $GITWEB_CONFIG : $GITWEB_CONFIG_SYSTEM)]}'.
+</p>
+EOT
+	die_error(500, 'Internal server error', $err_msg);
+}
+
 $projects_list ||= $projectroot;
 
 if (defined $maxload && get_loadavg() > $maxload) {
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 5a734b1..66a3e2d 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -26,6 +26,7 @@ our \$projects_list = '';
 our \$export_ok = '';
 our \$strict_export = '';
 our \$maxload = undef;
+our \$git_versions_must_match = 0;
 
 EOF
 
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 7590f10..e195f97 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -133,5 +133,32 @@ cat >>gitweb_config.perl <<\EOF
 our $maxload = undef;
 EOF
 
+# ======================================================================
+# check $git_versions_must_match feature
+# should be last section, just in case
+cp -f gitweb_config.perl gitweb_config.perl.bak
+echo 'our $git_versions_must_match = 1;' >>gitweb_config.perl
+
+cat <<\EOF >>gitweb_config.perl
+our $version = "current";
+EOF
+test_expect_success 'force version match: no match' '
+	gitweb_run "p=.git" &&
+	grep "Status: 500 Internal Server Error" gitweb.headers &&
+	grep "500 - Internal server error" gitweb.body
+'
+test_debug 'cat gitweb.headers'
+
+cat <<\EOF >>gitweb_config.perl
+# must be kept in sync with code in gitweb/gitweb.perl
+our $version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
+EOF
+test_expect_success 'force version match: match' '
+	gitweb_run "p=.git" &&
+	grep "Status: 200 OK" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+mv -f gitweb_config.perl.bak gitweb_config.perl
 
 test_done
-- 
1.6.6

^ permalink raw reply related

* [PATCH 0/2] gitweb misc fixes mkII
From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw)
  To: git

This is a new patch series dealing with the slightly controversial
option to force version matching, and this adds a new patch that solves 
a minor issue discovered on kernel.org.

John 'Warthog9' Hawley (2):
  gitweb: Add an option to force version match
  gitweb: Fix chop_str to allow for & characters in strings

 gitweb/README                            |    5 ++++
 gitweb/gitweb.perl                       |   33 ++++++++++++++++++++++++++++-
 t/gitweb-lib.sh                          |    1 +
 t/t9501-gitweb-standalone-http-status.sh |   27 ++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 2 deletions(-)

^ permalink raw reply

* [PATCH 2/2] gitweb: Fix chop_str to allow for & characters in strings
From: John 'Warthog9' Hawley @ 2010-02-02 21:56 UTC (permalink / raw)
  To: git
In-Reply-To: <1265147814-13284-2-git-send-email-warthog9@eaglescrag.net>

I'm unsure why this was placed in their originally, and it seems to
ultimately stem from code from before gitweb was merged into git core,
but there's an instance where git chops a string incorrectly based on
this.

Specifically:

	API & protocol: support option to force written data immediately to disk

from http://git.kernel.org/?p=daemon/distsrv/chunkd.git;a=commit;h=3b02f749df2cb1288f345a689d85e7061f507e54

The short version of the title gets chopped to

	API ...

where it should be

	API & protocol: support option to force written data...

This reverts that specific problem.
---
 gitweb/gitweb.perl |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 57771a0..4cc6d19 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1382,7 +1382,6 @@ sub chop_str {
 		my $body = $1;
 		my $tail = $2;
 		if (length($tail) > 4) {
-			$body =~ s/&[^;]*$//;
 			$tail = "... ";
 		}
 		return "$body$tail";
-- 
1.6.6

^ permalink raw reply related

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 21:56 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Avery Pennarun, Jacob Helwig, git
In-Reply-To: <394FB581-C9B9-40AB-AFB6-39B68ED5BEAD@wincent.com>

>
> As has already been pointed out, an extremely common workflow for "git
> commit --amend" is to include additional changes that were accidentally
> overlooked  in the initial commit, and no changes are made to the commit
> message.
>
> Changing the behavior would break that perfectly reasonable,
> widely-practised workflow, and _that_ would be 100% not ok.

I'm not against that workflow - I'm not forcing anybody to change the
commit message. The thing I'm talking about is roughly that it should
NOT treat the result of command ":q" the same way as the command
":wq".

For example IF there is a variable somewhere where message is stored:
For new commit message it is NULL and when commit is ready it is set
up to "message". We check If it is not empty and not NULL (:wq
executed),  then we can commit.
For "prepopulated" messages this variable can be is set to "message"
from the beginning, so it doesn't matter if you're writing changes or
not the check will always be true, so we can commit.
If the implementation would be like this, then I would propose to
clean up this variable after the prepopulated message is populated, so
it won't use it blindly, but only after you're overwriting the message
(even if the message is not changed).

^ permalink raw reply

* Re: Completion of error handling
From: Markus Elfring @ 2010-02-02 21:55 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Nicolas Pitre, git
In-Reply-To: <32541b131002021327u29e666b8ia5c6aab25fb55688@mail.gmail.com>


>
> If you never notice, then that's a good thing, right? :)
>   

Well, this depends on the kind of potential damage that will come from
the ignorance of function return values.
It might hurt more when corresponding effects can not be ignorable any
more because they became so obvious.

Regards,
Markus

^ permalink raw reply

* Re: Fix signal handler
From: Markus Elfring @ 2010-02-02 21:44 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20100202205849.GA14385@sigill.intra.peff.net>


>
> No, it's not a sig_atomic_t, but it is assignment of a single function
> pointer that is properly declared as volatile. Is this actually a
> problem on any known system?
>   

Is it guaranteed to work on all supported software environments that an
address can be atomically set?


> If you want to nit-pick, there are much worse cases. For example, in
> diff.c, we do quite a bit of work in remove_tempfile_on_signal.
>   

Thanks that you point out another open issue.


> It assumes that char* assignment is atomic, but nothing is even marked as
> volatile. But again, is this actually a problem on any system?
>   

Would you like to provide software implementations that work by design?

Regards,
Markus

^ permalink raw reply

* Re: Completion of error handling
From: Avery Pennarun @ 2010-02-02 21:27 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Nicolas Pitre, git
In-Reply-To: <4B68989D.80906@web.de>

On Tue, Feb 2, 2010 at 4:26 PM, Markus Elfring <Markus.Elfring@web.de> wrote:
>> ...then it will be easy to get people to accept your patches to fix
>> the error handling.
>
> You do not need to make assumptions about error situations. Please check
> all error codes.
> Otherwise, it might not be noticed if anything unexpected went wrong.

If you never notice, then that's a good thing, right? :)

Avery

^ permalink raw reply

* Re: Completion of error handling
From: Markus Elfring @ 2010-02-02 21:26 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Nicolas Pitre, git
In-Reply-To: <32541b131002021225k19ef94b2me8ef40cd502b85d@mail.gmail.com>


>
> ...then it will be easy to get people to accept your patches to fix
> the error handling.
>   

You do not need to make assumptions about error situations. Please check
all error codes.
Otherwise, it might not be noticed if anything unexpected went wrong.

Regards,
Markus

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Wincent Colaiuta @ 2010-02-02 20:58 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Avery Pennarun, Jacob Helwig, git
In-Reply-To: <76c5b8581002021247j6df8f609ld9e5d87a060a5423@mail.gmail.com>

El 02/02/2010, a las 21:47, Eugene Sajine escribió:

> On Tue, Feb 2, 2010 at 3:27 PM, Avery Pennarun <apenwarr@gmail.com>  
> wrote:
>> On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig  
>> <jacob.helwig@gmail.com> wrote:
>>> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com>  
>>> wrote:
>>>> When git commit --amend is executed it fires up vi so i can  
>>>> change the
>>>> commit message. If I have something staged, then when I'm exiting  
>>>> vi
>>>> using :q (without doing/writing any changes), it still commits
>>>> everything staged with old message.
>>>>
>>>> I believe it should actually abort amending and return to the state
>>>> before the "git commit --amend" was issued.
>>>
>>> I don't think this is actually the right way to go.  A _very_ common
>>> use case for "commit --amend" is to add things to the previous  
>>> commit,
>>> without changing the commit message at all.  Yes, you can avoid  
>>> having
>>> to fire up the editor at all in this case, but it's still a  
>>> perfectly
>>> valid thing to want to do.
>>
>> I agree and I do this all the time. However, I've also done it
>> accidentally before I learned the "remove the commit message and  
>> save"
>> trick.
>>
>> Perhaps what's really missing is more documentation of how to
>> "unamend" if you change your mind :)  I happen to know about "git
>> reset HEAD@{1}" but it's not terribly obvious.
>>
>> Avery
>>
>
> Well, i understand this logic but it seems to me that the issue is a
> little bit deeper than that.
> I just realized that the "rebase -i" is doing the same thing.
>
> If you're just exiting the vi using :q without writing changes it will
> perform rebase! I'm 100% sure it is not OK.

As has already been pointed out, an extremely common workflow for "git  
commit --amend" is to include additional changes that were  
accidentally overlooked  in the initial commit, and no changes are  
made to the commit message.

Changing the behavior would break that perfectly reasonable, widely- 
practised workflow, and _that_ would be 100% not ok.

Lucky for you, you've already been provided with two means of aborting  
(empty commit message, and non-zero editor exit), so you might as well  
start getting used to them.

Wincent

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Avery Pennarun @ 2010-02-02 21:18 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: Jacob Helwig, git
In-Reply-To: <76c5b8581002021247j6df8f609ld9e5d87a060a5423@mail.gmail.com>

On Tue, Feb 2, 2010 at 3:47 PM, Eugene Sajine <euguess@gmail.com> wrote:
> If you're just exiting the vi using :q without writing changes it will
> perform rebase! I'm 100% sure it is not OK.
>
> So, it seems that it will do that every time it has some
> *prepopulated* message in the editor, so it doesn't understand when
> changes are written and when they are not.
>
> I believe all those actions should be treated exactly the same way as
> the commit itself. If I'm writing/saving changes - it is good to go.
> If not - abort.

It might be sort of neat to check the mtime of the commit message file
before and after launching the editor; then if you want, you could
force-save the file even without any changes, and git would know that
you want it changed.

Not sure I would like that behaviour better than the current way,
actually, but it's an idea.

Avery

^ permalink raw reply

* Re: [PATCH] git-gui: use themed tk widgets with Tk 8.5
From: Pat Thoyts @ 2010-02-02 21:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Shawn O. Pearce
In-Reply-To: <4B67E183.3010800@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

>Pat Thoyts schrieb:
>> This patch enables the use of themed Tk widgets with Tk 8.5 and above.
>
>With this patch, the sizes of the file lists are not restored correctly
>anymore. The upper list (unstage files) always appears in the "native"
>size. A fix for this would be appreciated.
>
The ttk::panedwindow has different sash handling to the non themed
version. I'll try and get onto this soon.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply

* Re: master^ is not a local branch -- huh?!?
From: tytso @ 2010-02-02 21:07 UTC (permalink / raw)
  To: Ron Garret; +Cc: git
In-Reply-To: <ron1-0A5B25.21231201022010@news.gmane.org>

On Mon, Feb 01, 2010 at 09:23:12PM -0800, Ron Garret wrote:
> That's fine.  My only aim here is to raise the issue.
> 
> By the way, if you (plural) think it would be helpful I'd be happy to 
> take a stab at rewriting this part of the manual.  Writing docs is a 
> drag, but it would probably be a useful exercise for me.

It's definitely helpful to have someone who is learning how things
works to point out deficiencies and (ideally) suggest improvements to
the documentation.  Most of us here either were around at the
beginning, or (like myself) have used git long enough that we *know*
how things works, and reading the manual with the eyes of a novice is
a skill that few experts have.  It's why tech writers are (well, should
be) paid the big bucks.  :-)

					- Ted

^ permalink raw reply

* Re: Fix signal handler
From: Jeff King @ 2010-02-02 20:58 UTC (permalink / raw)
  To: Markus Elfring; +Cc: git
In-Reply-To: <4B684F5F.7020409@web.de>

On Tue, Feb 02, 2010 at 05:14:23PM +0100, Markus Elfring wrote:

> The function "early_output" that is set as a signal handler by the
> function "setup_early_output" contains a simple looking instruction.
> [...]
> A global variable gets a function pointer assigned.
> [...]
> I find that this approach does not fit to standard rules because the
> data type "sig_atomic_t" is the only type that can be safely used for
> global write access in signal handlers.

No, it's not a sig_atomic_t, but it is assignment of a single function
pointer that is properly declared as volatile. Is this actually a
problem on any known system?

If you want to nit-pick, there are much worse cases. For example, in
diff.c, we do quite a bit of work in remove_tempfile_on_signal. It
assumes that char* assignment is atomic, but nothing is even marked as
volatile. But again, is this actually a problem on any system?

You will find that most git developers care about real problems that can
be demonstrated on real systems. Standards can be a useful guide, but
they can be too loose (e.g., we run on some non-POSIX systems) as well
as too restrictive. What matters is what actually runs in practice.

If you can demonstrate a practical problem and provide a patch, then I
am sure people would be happy to read it.

-Peff

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Eugene Sajine @ 2010-02-02 20:47 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Jacob Helwig, git, Eugene Sajine
In-Reply-To: <32541b131002021227o1ec9f369w6096e85382857b8a@mail.gmail.com>

On Tue, Feb 2, 2010 at 3:27 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
> On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig <jacob.helwig@gmail.com> wrote:
>> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
>>> When git commit --amend is executed it fires up vi so i can change the
>>> commit message. If I have something staged, then when I'm exiting vi
>>> using :q (without doing/writing any changes), it still commits
>>> everything staged with old message.
>>>
>>> I believe it should actually abort amending and return to the state
>>> before the "git commit --amend" was issued.
>>
>> I don't think this is actually the right way to go.  A _very_ common
>> use case for "commit --amend" is to add things to the previous commit,
>> without changing the commit message at all.  Yes, you can avoid having
>> to fire up the editor at all in this case, but it's still a perfectly
>> valid thing to want to do.
>
> I agree and I do this all the time. However, I've also done it
> accidentally before I learned the "remove the commit message and save"
> trick.
>
> Perhaps what's really missing is more documentation of how to
> "unamend" if you change your mind :)  I happen to know about "git
> reset HEAD@{1}" but it's not terribly obvious.
>
> Avery
>

Well, i understand this logic but it seems to me that the issue is a
little bit deeper than that.
I just realized that the "rebase -i" is doing the same thing.

If you're just exiting the vi using :q without writing changes it will
perform rebase! I'm 100% sure it is not OK.

So, it seems that it will do that every time it has some
*prepopulated* message in the editor, so it doesn't understand when
changes are written and when they are not.

I believe all those actions should be treated exactly the same way as
the commit itself. If I'm writing/saving changes - it is good to go.
If not - abort.
I.e. the message can be prepopulated in the editor, but it should be
the same state as when you create a new message for commit, i.e. git
should not know anything about that message.

Thanks,
Eugene

^ permalink raw reply

* update kernel with git
From: james_68 @ 2010-02-02 20:47 UTC (permalink / raw)
  To: git


Hi, In my git repository I have a linux kernel 2.6.31 with a branch, I have
to update master branch to 2.6.32.7. 
I tried git pull path_2.6.32 master but it doesn't run, it does some
computation and then it says:
......
walk df56cb1f37415f16a187738ee9e3c03e2106e305
From http://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.32.y
 * branch            master     -> FETCH_HEAD
error: Entry '.gitignore' not uptodate. Cannot merge.
fatal: merging of trees 44927d86ace5c502486e26ed828a02fea9627119 and
84637ae413afcb236f0e9060e03cdec9fa017b0b failed

The question is: Is it the correct way execute git pull .... to update my
2.6.31 git kernel?

Thanks
Bye

-- 
View this message in context: http://n2.nabble.com/update-kernel-with-git-tp4503539p4503539.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Avery Pennarun @ 2010-02-02 20:27 UTC (permalink / raw)
  To: Jacob Helwig; +Cc: Eugene Sajine, git
In-Reply-To: <8c9a061002021214j673afbc8vfa5d941fba518648@mail.gmail.com>

On Tue, Feb 2, 2010 at 3:14 PM, Jacob Helwig <jacob.helwig@gmail.com> wrote:
> On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
>> When git commit --amend is executed it fires up vi so i can change the
>> commit message. If I have something staged, then when I'm exiting vi
>> using :q (without doing/writing any changes), it still commits
>> everything staged with old message.
>>
>> I believe it should actually abort amending and return to the state
>> before the "git commit --amend" was issued.
>
> I don't think this is actually the right way to go.  A _very_ common
> use case for "commit --amend" is to add things to the previous commit,
> without changing the commit message at all.  Yes, you can avoid having
> to fire up the editor at all in this case, but it's still a perfectly
> valid thing to want to do.

I agree and I do this all the time. However, I've also done it
accidentally before I learned the "remove the commit message and save"
trick.

Perhaps what's really missing is more documentation of how to
"unamend" if you change your mind :)  I happen to know about "git
reset HEAD@{1}" but it's not terribly obvious.

Avery

^ permalink raw reply

* Re: Completion of error handling
From: Avery Pennarun @ 2010-02-02 20:25 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Nicolas Pitre, git
In-Reply-To: <4B6886CE.9040602@web.de>

On Tue, Feb 2, 2010 at 3:10 PM, Markus Elfring <Markus.Elfring@web.de> wrote:
>> This isn't an opinion; because it's open source, you can actually look
>> at the source code and find out that many system calls don't do memory
>> allocation at all.
>
> I agree to your view because you distinguish between the available
> software implementations here.
> Are you also interested to complete error detection and corresponding
> exception handling?

I think if you can demonstrate actual situations where lack of error
handling could cause:
a) user confusion; or
b) data loss/corruption

...then it will be easy to get people to accept your patches to fix
the error handling.  If you can't reproduce or at least describe such
a case in detail, however, people probably won't be too excited about
it.  git developers seem to concentrate more on concrete problems than
theoretical ones.

Avery

^ permalink raw reply

* Re: Completion of error handling
From: Markus Elfring @ 2010-02-02 20:10 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Nicolas Pitre, git
In-Reply-To: <32541b131002021149g6ffe703erd9e25621e4305a52@mail.gmail.com>


>
> This isn't an opinion; because it's open source, you can actually look
> at the source code and find out that many system calls don't do memory
> allocation at all.
>   

I agree to your view because you distinguish between the available
software implementations here.
Are you also interested to complete error detection and corresponding
exception handling?

Regards,
Markus

^ permalink raw reply

* Re: [BUG] - "git commit --amend" commits, when exiting the editor  with no changes written
From: Jacob Helwig @ 2010-02-02 20:14 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: git
In-Reply-To: <76c5b8581002021207y3eccdc19i9a4abcc3d04315f0@mail.gmail.com>

On Tue, Feb 2, 2010 at 12:07, Eugene Sajine <euguess@gmail.com> wrote:
> Hi,
>
> It seems to me it is a bug:
>
> When git commit --amend is executed it fires up vi so i can change the
> commit message. If I have something staged, then when I'm exiting vi
> using :q (without doing/writing any changes), it still commits
> everything staged with old message.
>
> I believe it should actually abort amending and return to the state
> before the "git commit --amend" was issued.
>
> Thanks,
> Eugene
>

I don't think this is actually the right way to go.  A _very_ common
use case for "commit --amend" is to add things to the previous commit,
without changing the commit message at all.  Yes, you can avoid having
to fire up the editor at all in this case, but it's still a perfectly
valid thing to want to do.

If you want to abort amending, and return to the state before the "git
commit --amend", you can remove all non-comment lines (IE: write out
an "empty" commit message), or have your editor exit with a non-zero
exit code (Use ":cquit").

The comment block that comes up in your editor mentions this first
method, explicitly. "..., and an empty message aborts the commit."

-Jacob

^ permalink raw reply

* [BUG] - "git commit --amend" commits, when exiting the editor with no  changes written
From: Eugene Sajine @ 2010-02-02 20:07 UTC (permalink / raw)
  To: git

Hi,

It seems to me it is a bug:

When git commit --amend is executed it fires up vi so i can change the
commit message. If I have something staged, then when I'm exiting vi
using :q (without doing/writing any changes), it still commits
everything staged with old message.

I believe it should actually abort amending and return to the state
before the "git commit --amend" was issued.

Thanks,
Eugene

^ permalink raw reply

* Re: Linux Kernel based project in git
From: Avery Pennarun @ 2010-02-02 19:53 UTC (permalink / raw)
  To: git; +Cc: git
In-Reply-To: <9da7f2802f639777acfeb38eb1e3db90.squirrel@webmail.eisendle.net>

On Tue, Feb 2, 2010 at 4:05 AM,  <git@eisendle.net> wrote:
> For release we always generate 3 patches:
> - BSP patch
> - USB patch (since USB part is an external patch comming from a 3rd party)
> - WiFi patch (same as for USB)
>
> So my question is:
> What's the best way for handling this inside the git repository?
>
> IMHO it would make sense to have 3 branches (BSP, USB, WiFi) each based on
> unmodified 2.6.22 Kernel. USB and WiFi branch is used for generating the
> patch and for applying possible fixes. BSP branch for actual BSP related
> feature development and fixes.
> The changes in these branches are merged into the master branch which is
> used for compiling/testing the whole BSP.

Are you planning to submit these patches upstream at any point?  If
not, it might be easiest to just jam them all together in one branch
and not look back.  Since it seems like they probably affect quite
different parts of the code, you could always extract a clean set of
patches *later* and submit those patches upstream.

But that's just my lazy advice :)  The disadvantage to maintaining
them in separate branches is that probably none of the three branches
will work on its own anyway, since you don't have a physical device
that only has the new USB device, or only the new WiFi device, or only
needs the BSP but doesn't have updated USB or WiFi.  Putting them in
separate branches is therefore a bit artificial and won't buy you
much.

Have fun,

Avery

^ permalink raw reply

* Re: Completion of error handling
From: Avery Pennarun @ 2010-02-02 19:49 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Nicolas Pitre, git
In-Reply-To: <4B688042.8090400@web.de>

On Tue, Feb 2, 2010 at 2:42 PM, Markus Elfring <Markus.Elfring@web.de> wrote:
> Nicolas wrote:
>> At least on Linux, most of those functions simply cannot fail.
>
> Would you like to support software environments that work without a
> famous out-of-memory killer?

In many cases on Linux systems, you don't get the OOM notification
anyway until you try to *use* the memory, not at allocation time.  So
even checking the results of malloc() won't always save you (although
*not* checking can make problems even worse).

However, for functions that can't allocate memory at all, it's extra
pointless to worry about.

>> There is just no dynamic memory allocation involved.
>
> I find this opinion strange.

This isn't an opinion; because it's open source, you can actually look
at the source code and find out that many system calls don't do memory
allocation at all.

Avery

^ 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