[PATCH 0/5] git-gui: Remotes manipulation enhancements

[PATCH 0/5] git-gui: Remotes manipulation enhancements

[pasky]

This patch series implements various remotes manipulation enhancements,
hopefully eliminating the main remaining gap in git-gui functionality.

I hope this will go ok... the general atrocity of git send-email forced me to
actually send out this TopGit patch queue using quilt mail for now.

[PATCH 1/5] git-gui: Clarify the Remote -> Delete... action

[pasky]

[-- Attachment #1: t/git-gui/delbranch.diff --]
[-- Type: text/plain, Size: 1724 bytes --]

Currently, it was not really clear what all does this perform. We rename
"Delete..." to "Delete Branch..." (since this does not delete the remote
as a whole) and relabel the window from "Delete Remote Branch" to "Delete
Branch Remotely" (since the action also involves pushing the delete out).

This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/git-gui.sh                   |    2 +-
 git-gui/lib/remote_branch_delete.tcl |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 10d8a44..355c782 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2309,7 +2309,7 @@ if {[is_enabled transport]} {
 		-command do_push_anywhere \
 		-accelerator $M1T-P
 	.mbar.remote add command \
-		-label [mc "Delete..."] \
+		-label [mc "Delete Branch..."] \
 		-command remote_branch_delete::dialog
 }
 
diff --git a/git-gui/lib/remote_branch_delete.tcl b/git-gui/lib/remote_branch_delete.tcl
index c7b8148..fbcfb27 100644
--- a/git-gui/lib/remote_branch_delete.tcl
+++ b/git-gui/lib/remote_branch_delete.tcl
@@ -26,12 +26,12 @@ constructor dialog {} {
 	global all_remotes M1B
 
 	make_toplevel top w
-	wm title $top [append "[appname] ([reponame]): " [mc "Delete Remote Branch"]]
+	wm title $top [append "[appname] ([reponame]): " [mc "Delete Branch Remotely"]]
 	if {$top ne {.}} {
 		wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
 	}
 
-	label $w.header -text [mc "Delete Remote Branch"] -font font_uibold
+	label $w.header -text [mc "Delete Branch Remotely"] -font font_uibold
 	pack $w.header -side top -fill x
 
 	frame $w.buttons
-- 
tg: (c427559..) t/git-gui/delbranch (depends on: vanilla/master)

[PATCH 2/5] git-gui: Squash populate_{push,fetch}_menu to populate_remotes_menu

[pasky]

[-- Attachment #1: t/git-gui/populate-refactor.diff --]
[-- Type: text/plain, Size: 4505 bytes --]

The meat of the routines is now separated to add_fetch_entry() and
add_push_entry(). This refactoring will allow easy implementation of adding
individual remotes later.

This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/git-gui.sh     |    3 +-
 git-gui/lib/remote.tcl |  137 ++++++++++++++++++++++++------------------------
 2 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 10d8a44..75bd460 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -3100,8 +3100,7 @@ if {[is_enabled transport]} {
 	load_all_remotes
 
 	set n [.mbar.remote index end]
-	populate_push_menu
-	populate_fetch_menu
+	populate_remotes_menu
 	set n [expr {[.mbar.remote index end] - $n}]
 	if {$n > 0} {
 		if {[.mbar.remote type 0] eq "tearoff"} { incr n }
diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl
index 0e86dda..d97c851 100644
--- a/git-gui/lib/remote.tcl
+++ b/git-gui/lib/remote.tcl
@@ -132,91 +132,92 @@ proc load_all_remotes {} {
 	set all_remotes [lsort -unique $all_remotes]
 }
 
-proc populate_fetch_menu {} {
-	global all_remotes repo_config
-
+proc add_fetch_entry {r} {
+	global repo_config
 	set remote_m .mbar.remote
 	set fetch_m $remote_m.fetch
 	set prune_m $remote_m.prune
-
-	foreach r $all_remotes {
-		set enable 0
-		if {![catch {set a $repo_config(remote.$r.url)}]} {
-			if {![catch {set a $repo_config(remote.$r.fetch)}]} {
-				set enable 1
-			}
-		} else {
-			catch {
-				set fd [open [gitdir remotes $r] r]
-				while {[gets $fd n] >= 0} {
-					if {[regexp {^Pull:[ \t]*([^:]+):} $n]} {
-						set enable 1
-						break
-					}
+	set enable 0
+	if {![catch {set a $repo_config(remote.$r.url)}]} {
+		if {![catch {set a $repo_config(remote.$r.fetch)}]} {
+			set enable 1
+		}
+	} else {
+		catch {
+			set fd [open [gitdir remotes $r] r]
+			while {[gets $fd n] >= 0} {
+				if {[regexp {^Pull:[ \t]*([^:]+):} $n]} {
+					set enable 1
+					break
 				}
-				close $fd
 			}
+			close $fd
 		}
+	}
 
-		if {$enable} {
-			if {![winfo exists $fetch_m]} {
-				menu $prune_m
-				$remote_m insert 0 cascade \
-					-label [mc "Prune from"] \
-					-menu $prune_m
-
-				menu $fetch_m
-				$remote_m insert 0 cascade \
-					-label [mc "Fetch from"] \
-					-menu $fetch_m
-			}
-
-			$fetch_m add command \
-				-label $r \
-				-command [list fetch_from $r]
-			$prune_m add command \
-				-label $r \
-				-command [list prune_from $r]
+	if {$enable} {
+		if {![winfo exists $fetch_m]} {
+			menu $prune_m
+			$remote_m insert 0 cascade \
+				-label [mc "Prune from"] \
+				-menu $prune_m
+
+			menu $fetch_m
+			$remote_m insert 0 cascade \
+				-label [mc "Fetch from"] \
+				-menu $fetch_m
 		}
+
+		$fetch_m add command \
+			-label $r \
+			-command [list fetch_from $r]
+		$prune_m add command \
+			-label $r \
+			-command [list prune_from $r]
 	}
 }
 
-proc populate_push_menu {} {
-	global all_remotes repo_config
-
+proc add_push_entry {r} {
+	global repo_config
 	set remote_m .mbar.remote
 	set push_m $remote_m.push
-
-	foreach r $all_remotes {
-		set enable 0
-		if {![catch {set a $repo_config(remote.$r.url)}]} {
-			if {![catch {set a $repo_config(remote.$r.push)}]} {
-				set enable 1
-			}
-		} else {
-			catch {
-				set fd [open [gitdir remotes $r] r]
-				while {[gets $fd n] >= 0} {
-					if {[regexp {^Push:[ \t]*([^:]+):} $n]} {
-						set enable 1
-						break
-					}
+	set enable 0
+	if {![catch {set a $repo_config(remote.$r.url)}]} {
+		if {![catch {set a $repo_config(remote.$r.push)}]} {
+			set enable 1
+		}
+	} else {
+		catch {
+			set fd [open [gitdir remotes $r] r]
+			while {[gets $fd n] >= 0} {
+				if {[regexp {^Push:[ \t]*([^:]+):} $n]} {
+					set enable 1
+					break
 				}
-				close $fd
 			}
+			close $fd
 		}
+	}
 
-		if {$enable} {
-			if {![winfo exists $push_m]} {
-				menu $push_m
-				$remote_m insert 0 cascade \
-					-label [mc "Push to"] \
-					-menu $push_m
-			}
-
-			$push_m add command \
-				-label $r \
-				-command [list push_to $r]
+	if {$enable} {
+		if {![winfo exists $push_m]} {
+			menu $push_m
+			$remote_m insert 0 cascade \
+				-label [mc "Push to"] \
+				-menu $push_m
 		}
+
+		$push_m add command \
+			-label $r \
+			-command [list push_to $r]
+	}
+}
+
+proc populate_remotes_menu {} {
+	global all_remotes
+
+	foreach r $all_remotes {
+		add_fetch_entry $r
+		add_push_entry $r
 	}
 }
-- 
tg: (c427559..) t/git-gui/populate-refactor (depends on: vanilla/master)

[PATCH 3/5] git-gui: Add support for adding remotes

[pasky]

[-- Attachment #1: t/git-gui/remote-add.diff --]
[-- Type: text/plain, Size: 6707 bytes --]

When a remote is being added, it can also be automatically either fetched
or initialized and pushed; this patch adds capability for initializing
of local and ssh repositories. This also of course leaves a lot of space
for further customization features, like individually turning the
initialization phase on/off or tuning attributes of the remote
repository; I consider that out of scope of this patch, however.

This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/git-gui.sh         |    4 +
 git-gui/lib/remote.tcl     |   15 ++++
 git-gui/lib/remote_add.tcl |  190 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+), 0 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 75bd460..79d7597 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2305,6 +2305,10 @@ if {[is_enabled transport]} {
 	menu .mbar.remote
 
 	.mbar.remote add command \
+		-label [mc "Add..."] \
+		-command remote_add::dialog \
+		-accelerator $M1T-A
+	.mbar.remote add command \
 		-label [mc "Push..."] \
 		-command do_push_anywhere \
 		-accelerator $M1T-P
diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl
index d97c851..643f0bc 100644
--- a/git-gui/lib/remote.tcl
+++ b/git-gui/lib/remote.tcl
@@ -221,3 +221,18 @@ proc populate_remotes_menu {} {
 		add_push_entry $r
 	}
 }
+
+proc add_single_remote {name location} {
+	global all_remotes repo_config
+	lappend all_remotes $name
+
+	git remote add $name $location
+
+	# XXX: Better re-read the config so that we will never get out
+	# of sync with git remote implementation?
+	set repo_config(remote.$name.url) $location
+	set repo_config(remote.$name.fetch) "+refs/heads/*:refs/remotes/$name/*"
+
+	add_fetch_entry $name
+	add_push_entry $name
+}
diff --git a/git-gui/lib/remote_add.tcl b/git-gui/lib/remote_add.tcl
new file mode 100644
index 0000000..89e88ee
--- /dev/null
+++ b/git-gui/lib/remote_add.tcl
@@ -0,0 +1,190 @@
+# git-gui remote adding support
+# Copyright (C) 2008 Petr Baudis
+
+class remote_add {
+
+field w              ; # widget path
+field w_name         ; # new remote name widget
+field w_loc          ; # new remote location widget
+
+field name         {}; # name of the remote the user has chosen
+field location     {}; # location of the remote the user has chosen
+
+field opt_action fetch; # action to do after registering the remote locally
+
+constructor dialog {} {
+	global repo_config
+
+	make_toplevel top w
+	wm title $top [append "[appname] ([reponame]): " [mc "Add Remote"]]
+	if {$top ne {.}} {
+		wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
+	}
+
+	label $w.header -text [mc "Add New Remote"] -font font_uibold
+	pack $w.header -side top -fill x
+
+	frame $w.buttons
+	button $w.buttons.create -text [mc Add] \
+		-default active \
+		-command [cb _add]
+	pack $w.buttons.create -side right
+	button $w.buttons.cancel -text [mc Cancel] \
+		-command [list destroy $w]
+	pack $w.buttons.cancel -side right -padx 5
+	pack $w.buttons -side bottom -fill x -pady 10 -padx 10
+
+	labelframe $w.desc -text [mc "Remote Details"]
+
+	label $w.desc.name_l -text [mc "Name:"]
+	set w_name $w.desc.name_t
+	entry $w_name \
+		-borderwidth 1 \
+		-relief sunken \
+		-width 40 \
+		-textvariable @name \
+		-validate key \
+		-validatecommand [cb _validate_name %d %S]
+	grid $w.desc.name_l $w_name -sticky we -padx {0 5}
+
+	label $w.desc.loc_l -text [mc "Location:"]
+	set w_loc $w.desc.loc_t
+	entry $w_loc \
+		-borderwidth 1 \
+		-relief sunken \
+		-width 40 \
+		-textvariable @location
+	grid $w.desc.loc_l $w_loc -sticky we -padx {0 5}
+
+	grid columnconfigure $w.desc 1 -weight 1
+	pack $w.desc -anchor nw -fill x -pady 5 -padx 5
+
+	labelframe $w.action -text [mc "Further Action"]
+
+	radiobutton $w.action.fetch \
+		-text [mc "Fetch Immediately"] \
+		-value fetch \
+		-variable @opt_action
+	pack $w.action.fetch -anchor nw
+
+	radiobutton $w.action.push \
+		-text [mc "Initialize Remote Repository and Push"] \
+		-value push \
+		-variable @opt_action
+	pack $w.action.push -anchor nw
+
+	radiobutton $w.action.none \
+		-text [mc "Do Nothing Else Now"] \
+		-value none \
+		-variable @opt_action
+	pack $w.action.none -anchor nw
+
+	grid columnconfigure $w.action 1 -weight 1
+	pack $w.action -anchor nw -fill x -pady 5 -padx 5
+
+	bind $w <Visibility> [cb _visible]
+	bind $w <Key-Escape> [list destroy $w]
+	bind $w <Key-Return> [cb _add]\;break
+	tkwait window $w
+}
+
+method _add {} {
+	global repo_config env
+	global M1B
+
+	if {$name eq {}} {
+		tk_messageBox \
+			-icon error \
+			-type ok \
+			-title [wm title $w] \
+			-parent $w \
+			-message [mc "Please supply a remote name."]
+		focus $w_name
+		return
+	}
+
+	# XXX: We abuse check-ref-format here, but
+	# that should be ok.
+	if {[catch {git check-ref-format "remotes/$name"}]} {
+		tk_messageBox \
+			-icon error \
+			-type ok \
+			-title [wm title $w] \
+			-parent $w \
+			-message [mc "'%s' is not an acceptable remote name." $name]
+		focus $w_name
+		return
+	}
+
+	if {[catch {add_single_remote $name $location}]} {
+		tk_messageBox \
+			-icon error \
+			-type ok \
+			-title [wm title $w] \
+			-parent $w \
+			-message [mc "Failed to add remote '%s' of location '%s'." $name $location]
+		focus $w_name
+		return
+	}
+
+	switch -- $opt_action {
+	fetch {
+		set c [console::new \
+			[mc "fetch %s" $remote] \
+			[mc "Fetching the %s" $remote]]
+		console::exec $c [list git fetch --all $name]
+	}
+	push {
+		set cmds [list]
+
+		# Parse the location
+		if { [regexp {(?:git\+)?ssh://([^/]+)(/.+)} $location xx host path]
+		     || [regexp {([^:][^:]+):(.+)} $location xx host path]} {
+			set ssh ssh
+			if {[info exists env(GIT_SSH)]} {
+				set ssh $env(GIT_SSH)
+			}
+			lappend cmds [list exec $ssh $host git --git-dir=$path init --bare]
+		} elseif { ! [regexp {://} $location xx] } {
+			lappend cmds [list exec git --git-dir=$location init --bare]
+		} else {
+			tk_messageBox \
+				-icon error \
+				-type ok \
+				-title [wm title $w] \
+				-parent $w \
+				-message [mc "Do not know how to initialize repository at location '%s'." $location]
+			destroy $w
+			return
+		}
+
+		set c [console::new \
+			[mc "push %s" $name] \
+			[mc "Setting up the %s (at %s)" $name $location]]
+
+		lappend cmds [list exec git push -v --all $name]
+		console::chain $c $cmds
+	}
+	none {
+	}
+	}
+
+	destroy $w
+}
+
+method _validate_name {d S} {
+	if {$d == 1} {
+		if {[regexp {[~^:?*\[\0- ]} $S]} {
+			return 0
+		}
+	}
+	return 1
+}
+
+method _visible {} {
+	grab $w
+	$w_name icursor end
+	focus $w_name
+}
+
+}
-- 
tg: (fd699bc..) t/git-gui/remote-add (depends on: t/git-gui/populate-refactor)

[PATCH] git-gui: Fix fetching from remotes when adding them

[Petr Baudis]

As you can see, this particular code branch did not see a lot
of testing for some time now. Apologies for that.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/lib/remote_add.tcl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/lib/remote_add.tcl b/git-gui/lib/remote_add.tcl
index 8e3ad16..fb29422 100644
--- a/git-gui/lib/remote_add.tcl
+++ b/git-gui/lib/remote_add.tcl
@@ -130,9 +130,9 @@ method _add {} {
 	switch -- $opt_action {
 	fetch {
 		set c [console::new \
-			[mc "fetch %s" $remote] \
-			[mc "Fetching the %s" $remote]]
-		console::exec $c [list git fetch --all $name]
+			[mc "fetch %s" $name] \
+			[mc "Fetching the %s" $name]]
+		console::exec $c [list git fetch $name]
 	}
 	push {
 		set cmds [list]
-- 
tg: (17f0c43..) t/git-gui/remote-fetch (depends on: git-gui/remotes)

[PATCH 4/5] git-gui: Add support for removing remotes

[pasky]

[-- Attachment #1: t/git-gui/remote-rm.diff --]
[-- Type: text/plain, Size: 2173 bytes --]

We introduce new submenu Remote -> Remove Remote, allowing to remove
remotes. In the future, we might consider a confirmation popup to avoid
misclicks, but removing a remote is not very lossy operation.

This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/lib/remote.tcl |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl
index 643f0bc..1852247 100644
--- a/git-gui/lib/remote.tcl
+++ b/git-gui/lib/remote.tcl
@@ -137,6 +137,7 @@ proc add_fetch_entry {r} {
 	set remote_m .mbar.remote
 	set fetch_m $remote_m.fetch
 	set prune_m $remote_m.prune
+	set remove_m $remote_m.remove
 	set enable 0
 	if {![catch {set a $repo_config(remote.$r.url)}]} {
 		if {![catch {set a $repo_config(remote.$r.fetch)}]} {
@@ -157,6 +158,11 @@ proc add_fetch_entry {r} {
 
 	if {$enable} {
 		if {![winfo exists $fetch_m]} {
+			menu $remove_m
+			$remote_m insert 0 cascade \
+				-label [mc "Remove Remote"] \
+				-menu $remove_m
+
 			menu $prune_m
 			$remote_m insert 0 cascade \
 				-label [mc "Prune from"] \
@@ -174,6 +180,9 @@ proc add_fetch_entry {r} {
 		$prune_m add command \
 			-label $r \
 			-command [list prune_from $r]
+		$remove_m add command \
+			-label $r \
+			-command [list remove_remote $r]
 	}
 }
 
@@ -236,3 +245,31 @@ proc add_single_remote {name location} {
 	add_fetch_entry $name
 	add_push_entry $name
 }
+
+proc delete_from_menu {menu name} {
+	if {[winfo exists $menu]} {
+		$menu delete $name
+	}
+}
+
+proc remove_remote {name} {
+	global all_remotes repo_config
+
+	git remote rm $name
+
+	catch {
+		# Missing values are ok
+		unset repo_config(remote.$name.url)
+		unset repo_config(remote.$name.fetch)
+		unset repo_config(remote.$name.push)
+	}
+
+	set i [lsearch -exact all_remotes $name]
+	lreplace all_remotes $i $i
+
+	set remote_m .mbar.remote
+	delete_from_menu $remote_m.fetch $name
+	delete_from_menu $remote_m.prune $name
+	delete_from_menu $remote_m.remove $name
+	delete_from_menu $remote_m.push $name
+}
-- 
tg: (f30d624..) t/git-gui/remote-rm (depends on: t/git-gui/remote-add)

[PATCH] Fix removing non-pushable remotes

[Petr Baudis]

Git-gui does not add most of the remotes to the 'push' menu
since they are missing the "Push" line in their remotespec.
In that case, removing the remote would end up with an error.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---

Note that I think this should be abandoned and all remotes
added to the push menu. Having the push part of the remotespec
is actually a very rare situation and it is just not there
in most of the cases, relying on default git push behaviour.

 git-gui/lib/remote.tcl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-gui/lib/remote.tcl b/git-gui/lib/remote.tcl
index 1852247..b92b429 100644
--- a/git-gui/lib/remote.tcl
+++ b/git-gui/lib/remote.tcl
@@ -271,5 +271,6 @@ proc remove_remote {name} {
 	delete_from_menu $remote_m.fetch $name
 	delete_from_menu $remote_m.prune $name
 	delete_from_menu $remote_m.remove $name
-	delete_from_menu $remote_m.push $name
+	# Not all remotes are in the push menu
+	catch { delete_from_menu $remote_m.push $name }
 }
-- 
tg: (17f0c43..) t/git-gui/remove-push (depends on: git-gui/remotes)

[PATCH 5/5] git-gui: mkdir -p when initializing new remote repository

[pasky]

[-- Attachment #1: t/git-gui/remote-mkdir.diff --]
[-- Type: text/plain, Size: 1070 bytes --]

This allows the user to create repositories with arbitrary paths
on the server. The downside is that errorneously typed paths are
not caught but instead created remotely; YMMV.

This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
 git-gui/lib/remote_add.tcl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-gui/lib/remote_add.tcl b/git-gui/lib/remote_add.tcl
index 89e88ee..8e3ad16 100644
--- a/git-gui/lib/remote_add.tcl
+++ b/git-gui/lib/remote_add.tcl
@@ -144,8 +144,9 @@ method _add {} {
 			if {[info exists env(GIT_SSH)]} {
 				set ssh $env(GIT_SSH)
 			}
-			lappend cmds [list exec $ssh $host git --git-dir=$path init --bare]
+			lappend cmds [list exec $ssh $host mkdir -p $location && git --git-dir=$path init --bare]
 		} elseif { ! [regexp {://} $location xx] } {
+			lappend cmds [list exec mkdir -p $location]
 			lappend cmds [list exec git --git-dir=$location init --bare]
 		} else {
 			tk_messageBox \
-- 
tg: (f30d624..) t/git-gui/remote-mkdir (depends on: t/git-gui/remote-add)

On Sponsor Notices

[Petr Baudis]

  Hi,

  to follow up a little on the "This patch has been sponsored by
Novartis" messages - I have been on a summer internship at Novartis busy
deploying Git and these patches (still quite a few more to come, mostly
for gitweb) have been one of the main outputs of that work.

  However, I'm not sure if acknowledging the Novartis-originated patches
in the log message like this is the best practice and we will understand
if the maintainers will decide to strip these notices when applying the
patches. Usually, this kind of acknowledgement is made by using
"sponsored" email addresses, however mine will probably stop working
shortly after I leave and the only way to read it is, shall we say,
utmostly inconvenient. ;-) Now, Shawn has proposed 'Sponsored-by:' line
at the header footer, which is also an interesting possibility.

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

Re: On Sponsor Notices

[Heikki Orsila]

On Thu, Sep 25, 2008 at 12:51:20AM +0200, Petr Baudis wrote:
> Now, Shawn has proposed 'Sponsored-by:' line
> at the header footer, which is also an interesting possibility.

In my opinion, one should aim for minimum amount of technically 
irrelevant information. Extra lines cause penalty for reading logs.

-- 
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd

Re: On Sponsor Notices

[Martin Langhoff]

On Thu, Sep 25, 2008 at 10:51 AM, Petr Baudis <pasky@suse.cz> wrote:
>  to follow up a little on the "This patch has been sponsored by
> Novartis" messages - I have been on a summer internship at Novartis busy
> deploying Git and these patches (still quite a few more to come, mostly
> for gitweb) have been one of the main outputs of that work.

In my case, if the contribution is done at the behest of a client the
copyright is theirs (they paid for my work, and it's a "work for hire"
in legal terms). So

 - if the contribution is large, I tend to add a copyright line (see
git-cvsserver: copyright != authors, though that's out of date now)

 - they probably have to provide sign-off, so s-o-b line is appropriate

>  However, I'm not sure if acknowledging the Novartis-originated patches
> in the log message like this is the best practice and we will understand
> if the maintainers will decide to strip these notices when applying the
> patches.

AIUI, the signed-off-by line is meant to track this, and it serves
both legal purposes and a as recognition.

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Re: On Sponsor Notices

[Petr Baudis]

On Thu, Sep 25, 2008 at 11:43:52AM +1200, Martin Langhoff wrote:
> On Thu, Sep 25, 2008 at 10:51 AM, Petr Baudis <pasky@suse.cz> wrote:
> >  However, I'm not sure if acknowledging the Novartis-originated patches
> > in the log message like this is the best practice and we will understand
> > if the maintainers will decide to strip these notices when applying the
> > patches.
> 
> AIUI, the signed-off-by line is meant to track this, and it serves
> both legal purposes and a as recognition.

That works fine when it is signed off by employers using their company
e-mail, but that is not feasible in my case - what should be in the
Signed-off-line? Name of the company? What about the e-mail? Some
generic e-mail of the legal department? That doesn't really work well
with all the machinery developed around s-o-b lines, besides the
question does not seem to be by far that simple with such a huge
corporation.

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

Re: On Sponsor Notices

[Martin Langhoff]

On Thu, Sep 25, 2008 at 11:47 AM, Petr Baudis <pasky@suse.cz> wrote:
> That works fine when it is signed off by employers using their company
> e-mail, but that is not feasible in my case -

Don't know that much about your case... Whoever at Novartis that gave
the go-ahead to "develop and release the patches"?

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Re: On Sponsor Notices

[Nicolas Pitre]

On Thu, 25 Sep 2008, Petr Baudis wrote:

>   Hi,
> 
>   to follow up a little on the "This patch has been sponsored by
> Novartis" messages - I have been on a summer internship at Novartis busy
> deploying Git and these patches (still quite a few more to come, mostly
> for gitweb) have been one of the main outputs of that work.
> 
>   However, I'm not sure if acknowledging the Novartis-originated patches
> in the log message like this is the best practice and we will understand
> if the maintainers will decide to strip these notices when applying the
> patches. Usually, this kind of acknowledgement is made by using
> "sponsored" email addresses, however mine will probably stop working
> shortly after I leave and the only way to read it is, shall we say,
> utmostly inconvenient. ;-) Now, Shawn has proposed 'Sponsored-by:' line
> at the header footer, which is also an interesting possibility.

I'd suggest you do like some people working on the Linux kernel, i.e. 
use your employer's email address for the Signed-off-by line but use 
whatever address you prefer for the from/author line.


Nicolas

Re: On Sponsor Notices

[Petr Baudis]

On Wed, Sep 24, 2008 at 10:36:41PM -0400, Nicolas Pitre wrote:
> I'd suggest you do like some people working on the Linux kernel, i.e. 
> use your employer's email address for the Signed-off-by line but use 
> whatever address you prefer for the from/author line.

Even if the employer's email address is basically bogus? Can you point
to some examples of Linux kernel patches doing that, please?

				Petr "Pasky" Baudis

Re: On Sponsor Notices

[Nicolas Pitre]

On Thu, 25 Sep 2008, Petr Baudis wrote:

> On Wed, Sep 24, 2008 at 10:36:41PM -0400, Nicolas Pitre wrote:
> > I'd suggest you do like some people working on the Linux kernel, i.e. 
> > use your employer's email address for the Signed-off-by line but use 
> > whatever address you prefer for the from/author line.
> 
> Even if the employer's email address is basically bogus? Can you point
> to some examples of Linux kernel patches doing that, please?

See commit 6c3a158316.  If you send a mail to npitre@mvista.com at this 
point, it most likely won't reach me.


Nicolas



> 
> 				Petr "Pasky" Baudis
> 


Nicolas

Re: On Sponsor Notices

[Petr Baudis]

On Thu, Sep 25, 2008 at 10:20:23AM -0400, Nicolas Pitre wrote:
> On Thu, 25 Sep 2008, Petr Baudis wrote:
> 
> > On Wed, Sep 24, 2008 at 10:36:41PM -0400, Nicolas Pitre wrote:
> > > I'd suggest you do like some people working on the Linux kernel, i.e. 
> > > use your employer's email address for the Signed-off-by line but use 
> > > whatever address you prefer for the from/author line.
> > 
> > Even if the employer's email address is basically bogus? Can you point
> > to some examples of Linux kernel patches doing that, please?
> 
> See commit 6c3a158316.  If you send a mail to npitre@mvista.com at this 
> point, it most likely won't reach me.

Hmm, perhaps that makes the most sense then. I'm not sure if Shawn
already applied my patches or which part of them - they don't see to be
pushed out yet. Shawn, if it's convenient for you, could you please

	s/This patch has been sponsored by Novartis.

Signed-off-by: Petr Baudis <pasky@suse.cz>/Signed-off-by: Petr Baudis
<petr.baudis@novartis.com>/ them?

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

Re: On Sponsor Notices

[Shawn O. Pearce]

Petr Baudis <pasky@suse.cz> wrote:
> On Thu, Sep 25, 2008 at 10:20:23AM -0400, Nicolas Pitre wrote:
> > 
> > See commit 6c3a158316.  If you send a mail to npitre@mvista.com at this 
> > point, it most likely won't reach me.
> 
> Hmm, perhaps that makes the most sense then. I'm not sure if Shawn
> already applied my patches or which part of them - they don't see to be
> pushed out yet. Shawn, if it's convenient for you, could you please
> 
> 	s/This patch has been sponsored by Novartis.
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>/Signed-off-by: Petr Baudis
> <petr.baudis@novartis.com>/ them?

Yup.  I only pushed them to my pu branch because I suspected
something like this was going to happen.  ;-)

One filter-branch script coming up...

-- 
Shawn.

Re: On Sponsor Notices

[Andreas Ericsson]

Petr Baudis wrote:
>   Hi,
> 
>   to follow up a little on the "This patch has been sponsored by
> Novartis" messages - I have been on a summer internship at Novartis busy
> deploying Git and these patches (still quite a few more to come, mostly
> for gitweb) have been one of the main outputs of that work.
> 
>   However, I'm not sure if acknowledging the Novartis-originated patches
> in the log message like this is the best practice and we will understand
> if the maintainers will decide to strip these notices when applying the
> patches. Usually, this kind of acknowledgement is made by using
> "sponsored" email addresses, however mine will probably stop working
> shortly after I leave and the only way to read it is, shall we say,
> utmostly inconvenient. ;-) Now, Shawn has proposed 'Sponsored-by:' line
> at the header footer, which is also an interesting possibility.
> 

I like the "Sponsored-by" idea. I work for a company that sponsors quite
a lot, and I thoroughly enjoy the idea that I can one day go to my boss
and say "hey, our company name is clearly visible here. We've sent this
many patches that got accepted upstream", and that other companies can
see that too.

As for the legal S-o-b stuff, I'd say a combination like this:
Sponsored-by: Example <contact@example.com>
Signed-off-by: Casper Intern (for Example) <random@real.com>

should work wonderfully. I know the guys holding the money like to see
the company name so it's a good thing to do to get a company to sponsor
development further, while the S-o-b marks the person responsible for
posting the patches to the project.

Just my €0.02.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: On Sponsor Notices

[A.J. Rossini]

Petr -

I should have been clear!   What I meant to say was that it would be
nice to have things like our Novartis sponsorship in the README of
projects which are particular to the repository structure and client
which we (well, you :-), developed, and make them available, but I'd
not be certain I'd like to have such acknowledgement entered into the
git development proper (I'd prefer to think that the changes would
have been made anyway, we just accelerated them -- however for the
direct project code (non-general), that clearly should have in a
README (not license) sponsored by Novartis, as it's technically
pre-competitive (we'd love to hire people who in addition to quant and
science skills, know how to use it for statistical/mathematical
modeling, and/or improve it).

(BTW, if anyone is interested in the possibility of a similar summer
project in Basel, working for a certain large multinational Pharma
company, doing open source work, possibly around git or other
"information management projects", please let me know -- we can take
on undergrads and grad students who know programming, mathematics, and
statistics, or just one of those disciplines -- it's not too bad, just
ask Petr).


On Thu, Sep 25, 2008 at 12:51 AM, Petr Baudis <pasky@suse.cz> wrote:
>  Hi,
>
>  to follow up a little on the "This patch has been sponsored by
> Novartis" messages - I have been on a summer internship at Novartis busy
> deploying Git and these patches (still quite a few more to come, mostly
> for gitweb) have been one of the main outputs of that work.
>
>  However, I'm not sure if acknowledging the Novartis-originated patches
> in the log message like this is the best practice and we will understand
> if the maintainers will decide to strip these notices when applying the
> patches. Usually, this kind of acknowledgement is made by using
> "sponsored" email addresses, however mine will probably stop working
> shortly after I leave and the only way to read it is, shall we say,
> utmostly inconvenient. ;-) Now, Shawn has proposed 'Sponsored-by:' line
> at the header footer, which is also an interesting possibility.
>
> --
>                                Petr "Pasky" Baudis
> People who take cold baths never have rheumatism, but they have
> cold baths.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
best,
-tony

blindglobe@gmail.com
Muttenz, Switzerland.
"Commit early,commit often, and commit in a repository from which we
can easily roll-back your mistakes" (AJR, 4Jan05).

Drink Coffee:  Do stupid things faster with more energy!