Git development
 help / color / mirror / Atom feed
* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Martin von Zweigbergk @ 2012-12-05  3:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2yunn0e.fsf@alter.siamese.dyndns.org>

On Sat, Dec 1, 2012 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>> Slightly off topic, but another difference (or somehow another aspect
>>> of the same difference?) that has tripped me up a few times is that
>>> "git checkout $rev ." only affects added and modified files...
>
> "checkout $commit pathspec" has always been about ...

I suppose the "has always been" is meant to say that it's hard to
change at this point, not that it's more intuitive the way it works..?

> ...checking out the
> contents stored in the paths that match the pathspec from the named
> commit to the index and also o the working tree.

I think I have always thought that "git checkout $commit $pathspec"
would replace the section(s) of the tree defined by $pathspec. (I'm
using "tree" in the more general sense here, as I'm understood the
index is not stored as a tree.)

> When pathspec is "dir/", it does not match the directory whose name
> is "dir".  The pathspec matches the paths that store blobs under
> that directory.

Ah, right. Unlike "git reset dir/", IIUC.

More importantly, when is it desirable not to delete deleted entries?
I find it much easier to imagine uses a "git checkout $commit
$pathspec" that does delete deleted entries. It seems like this must
have been discussed in depth before, so feel free to point me to an
old thread.

If it doesn't seem too strange to you and others if I make "git reset
--hard [$commit] $pathspec" work just like had expected "git checkout
$commit $pathspec", I might look into that when I get some time.

> ...The "please
> remove everything in dir/" part is not the job of "checkout"; of
> course, you can do it as a separate step (e.g. "rm -fr dir/").

"rm -rf dir/" would of course delete everything in there, including
e.g. build artifacts....

^ permalink raw reply

* [PATCH v2] gitk: read and write a repository specific configuration file
From: Łukasz Stelmach @ 2012-12-05  0:49 UTC (permalink / raw)
  To: git; +Cc: paulus, gitster, Łukasz Stelmach
In-Reply-To: <1354483766-13925-1-git-send-email-stlman@poczta.fm>

Enable gitk read and write repository specific configuration
file: ".git/k" if the file exists. To make gitk use the local
file simply create one, e.g. with the touch(1) command.

This is very useful if one uses different views for different
repositories. Now there is no need to store all of them in
~/.gitk and make the views list needlesly long.

Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---

Same as before but rebased onto Paul's repository.

 gitk |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/gitk b/gitk
index 379582a..c6b7dc3 100755
--- a/gitk
+++ b/gitk
@@ -2703,7 +2703,7 @@ proc doprogupdate {} {
 
 proc savestuff {w} {
     global canv canv2 canv3 mainfont textfont uifont tabstop
-    global stuffsaved findmergefiles maxgraphpct
+    global stuffsaved findmergefiles maxgraphpct gitdir
     global maxwidth showneartags showlocalchanges
     global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment datetimeformat limitdiffs
@@ -2714,10 +2714,12 @@ proc savestuff {w} {
     if {$stuffsaved} return
     if {![winfo viewable .]} return
     catch {
-	if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
-	set f [open "~/.gitk-new" w]
+	set fn [expr [file exists [file join $gitdir k]] ? \
+		{[file join $gitdir k-new]} : {"~/.gitk-new"}]
+	if {[file exists $fn]} {file delete -force $fn}
+	set f [open $fn  w]
 	if {$::tcl_platform(platform) eq {windows}} {
-	    file attributes "~/.gitk-new" -hidden true
+	    catch {file attributes "~/.gitk-new" -hidden true}
 	}
 	puts $f [list set mainfont $mainfont]
 	puts $f [list set textfont $textfont]
@@ -2769,7 +2771,7 @@ proc savestuff {w} {
 	}
 	puts $f "}"
 	close $f
-	file rename -force "~/.gitk-new" "~/.gitk"
+	file rename -force $fn [regsub {\-new$} $fn {}]
     }
     set stuffsaved 1
 }
@@ -11723,7 +11725,14 @@ namespace import ::msgcat::mc
 ## And eventually load the actual message catalog
 ::msgcat::mcload $gitk_msgsdir
 
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+    show_error {} . [mc "Cannot find a git repository here."]
+    exit 1
+}
+
 catch {source ~/.gitk}
+catch {source [file join $gitdir k]}
 
 parsefont mainfont $mainfont
 eval font create mainfont [fontflags mainfont]
@@ -11740,12 +11749,6 @@ setui $uicolor
 
 setoptions
 
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-    show_error {} . [mc "Cannot find a git repository here."]
-    exit 1
-}
-
 set selecthead {}
 set selectheadid {}
 
-- 
1.7.8.6

^ permalink raw reply related

* [PATCH v2] gitk: add a checkbox to control the visibility of tags
From: Łukasz Stelmach @ 2012-12-05  0:48 UTC (permalink / raw)
  To: git; +Cc: paulus, gitster, Łukasz Stelmach
In-Reply-To: <7vwqwztv75.fsf@alter.siamese.dyndns.org>

Enable hiding of tags displayed in the tree as yellow labels.
If a repository is used together with a system like Gerrit
there may be quite a lot of tags used to control building
and there may be hardly any place left for commit subjects.

Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
Rebased onto Paul's repository.

 gitk |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/gitk b/gitk
index 379582a..46c1a7c 100755
--- a/gitk
+++ b/gitk
@@ -1754,7 +1754,7 @@ proc readrefs {} {
     global tagids idtags headids idheads tagobjid
     global otherrefids idotherrefs mainhead mainheadid
     global selecthead selectheadid
-    global hideremotes
+    global hideremotes hidetags
 
     foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
 	catch {unset $v}
@@ -1776,6 +1776,7 @@ proc readrefs {} {
 	    set headids($name) $id
 	    lappend idheads($id) $name
 	} elseif {[string match "tags/*" $name]} {
+	    if {$hidetags} continue
 	    # this lets refs/tags/foo^{} overwrite refs/tags/foo,
 	    # which is what we want since the former is the commit ID
 	    set name [string range $name 5 end]
@@ -2709,7 +2710,7 @@ proc savestuff {w} {
     global cmitmode wrapcomment datetimeformat limitdiffs
     global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor
     global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk
-    global hideremotes want_ttk
+    global hideremotes hidetags want_ttk
 
     if {$stuffsaved} return
     if {![winfo viewable .]} return
@@ -2732,6 +2733,7 @@ proc savestuff {w} {
 	puts $f [list set autosellen $autosellen]
 	puts $f [list set showneartags $showneartags]
 	puts $f [list set hideremotes $hideremotes]
+	puts $f [list set hidetags $hidetags]
 	puts $f [list set showlocalchanges $showlocalchanges]
 	puts $f [list set datetimeformat $datetimeformat]
 	puts $f [list set limitdiffs $limitdiffs]
@@ -10924,7 +10926,7 @@ proc create_prefs_page {w} {
 proc prefspage_general {notebook} {
     global NS maxwidth maxgraphpct showneartags showlocalchanges
     global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
-    global hideremotes want_ttk have_ttk
+    global hideremotes hidetags want_ttk have_ttk
 
     set page [create_prefs_page $notebook.general]
 
@@ -10947,6 +10949,9 @@ proc prefspage_general {notebook} {
     ${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \
 	-variable hideremotes
     grid x $page.hideremotes -sticky w
+    ${NS}::checkbutton $page.hidetags -text [mc "Hide tag labels"] \
+	-variable hidetags
+    grid x $page.hidetags -sticky w
 
     ${NS}::label $page.ddisp -text [mc "Diff display options"]
     grid $page.ddisp - -sticky w -pady 10
@@ -11048,7 +11053,7 @@ proc doprefs {} {
     global oldprefs prefstop showneartags showlocalchanges
     global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
     global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
-    global hideremotes want_ttk have_ttk
+    global hideremotes hidetags want_ttk have_ttk
 
     set top .gitkprefs
     set prefstop $top
@@ -11057,7 +11062,7 @@ proc doprefs {} {
 	return
     }
     foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
-		   limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+		   limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
 	set oldprefs($v) [set $v]
     }
     ttk_toplevel $top
@@ -11177,7 +11182,7 @@ proc prefscan {} {
     global oldprefs prefstop
 
     foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
-		   limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+		   limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
 	global $v
 	set $v $oldprefs($v)
     }
@@ -11191,7 +11196,7 @@ proc prefsok {} {
     global oldprefs prefstop showneartags showlocalchanges
     global fontpref mainfont textfont uifont
     global limitdiffs treediffs perfile_attrs
-    global hideremotes
+    global hideremotes hidetags
 
     catch {destroy $prefstop}
     unset prefstop
@@ -11237,7 +11242,8 @@ proc prefsok {} {
 	  $limitdiffs != $oldprefs(limitdiffs)} {
 	reselectline
     }
-    if {$hideremotes != $oldprefs(hideremotes)} {
+    if {$hideremotes != $oldprefs(hideremotes) ||
+        $hidetags != $oldprefs(hidetags)} {
 	rereadrefs
     }
 }
@@ -11661,6 +11667,7 @@ set cmitmode "patch"
 set wrapcomment "none"
 set showneartags 1
 set hideremotes 0
+set hidetags 0
 set maxrefs 20
 set maxlinelen 200
 set showlocalchanges 1
-- 
1.7.8.6

^ permalink raw reply related

* Re: [PATCH v2] submodule: add 'deinit' command
From: Junio C Hamano @ 2012-12-04 23:06 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Phil Hord, W. Trevor King, Git, Heiko Voigt, Jeff King,
	Shawn Pearce, Nahor
In-Reply-To: <50BE6FB9.70301@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> +If you only want to remove the local checkout of a submodule from your
> +work tree without committing that use `git submodule deinit` instead
> +(see linkgit:git-submodule[1]).

I'll add a comma between "without commiting that" and "use X
instead"; it will read better, I think.

> +test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
> +	git config submodule.example.foo bar &&
> +	git submodule deinit &&
> +	test -z "$(git config submodule.example.url)" &&
> +	test -z "$(git config submodule.example.foo)"
> +'

This is sufficient, but it might be cleaner to see if

    git config --get-regexp "^submodule\.example\."

results in empty.  Does not make much difference to warrant a re-roll.

> +test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
> +	git submodule deinit &&
> +	test_must_fail git submodule deinit example
> +'
> +
>  test_done

Thanks; will queue.

^ permalink raw reply

* [PATCH v2] submodule: add 'deinit' command
From: Jens Lehmann @ 2012-12-04 21:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Phil Hord, W. Trevor King, Git, Heiko Voigt, Jeff King,
	Shawn Pearce, Nahor
In-Reply-To: <7vhao31s9e.fsf@alter.siamese.dyndns.org>

With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config himself).

Help those users by providing a 'deinit' command. This removes the whole
submodule.<name> section from .git/config either for the given
submodule(s) or for all those which have been initialized if none were
given. Complain only when for a submodule given on the command line the
url setting can't be found in .git/config.

Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.

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

Am 03.12.2012 08:58, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Maybe the principle of least surprise is better followed when we
>> nuke the whole section, as it might surprise the user more to have
>> a setting resurrected he customized in the last life cycle of the
>> submodule than seeing that after an deinit followed by an init all
>> former customizations are consistently gone. So I tend to think now
>> that removing the whole section would be the better solution here.
> 
> I tend to agree; I suspect that a "deinit" would be mostly done
> either to
> 
>  (1) correct mistakes the user made during a recent "init" and
>      perhaps "sync"; or
> 
>  (2) tell Git that the user has finished woing with this particular
>      submodule and does not intend to use it for quite a while.
> 
> For both (1) and (2), I think it would be easier to users if we gave
> them a clean slate, the same state as the one the user who never had
> ran "init" on it would be in.  A user in situation (1) is asking for
> a clean slate, and a user in situation (2) is better served if he
> does not have to worry about leftover entries in $GIT_DIR/config he
> has long forgotten from many months ago (during which time the way
> the project uses the particular submodule may well have changed)
> giving non-standard experience different from what other project
> participants would get.

Changes in v2:
- Remove the whole submodule section instead of only removing the
  "url" setting and explain why we do that in a comment
- Reworded commit message and git-submodule.txt to reflect that
- Extend the test to check that a custom settings are removed


 Documentation/git-rm.txt        |  4 ++++
 Documentation/git-submodule.txt | 12 ++++++++++
 git-submodule.sh                | 52 ++++++++++++++++++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 12 ++++++++++
 4 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 262436b..ec42bf5 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing that use `git submodule deinit` instead
+(see linkgit:git-submodule[1]).
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b1de3ba..08b55a7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
@@ -134,6 +135,17 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.

+deinit::
+	Unregister the submodules, i.e. remove the whole `submodule.$name`
+	section from .git/config. Further calls to `git submodule update`,
+	`git submodule foreach` and `git submodule sync` will skip any
+	unregistered submodules until they are initialized again, so use
+	this command if you don't want to have a local checkout of the
+	submodule in your work tree anymore (but note that this command
+	does not remove the submodule work tree). If you really want to
+	remove a submodule from the repository and commit that use
+	linkgit:git-rm[1] instead.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.
diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..3f558ed 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
@@ -516,6 +517,55 @@ cmd_init()
 }

 #
+# Unregister submodules from .git/config
+#
+# $@ = requested paths (default to all)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... init".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		die_if_unmatched "$mode"
+		name=$(module_name "$sm_path") || exit
+		url=$(git config submodule."$name".url)
+		if test -z "$url"
+		then
+			# Only mention uninitialized submodules when its
+			# path have been specified
+			test "$#" != "0" &&
+			say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+			continue
+		fi
+		# Remove the whole section so we have a clean state when the user
+		# later decides to init this submodule again
+		git config --remove-section submodule."$name" &&
+		say "$(eval_gettext "Submodule '\$name' (\$url) unregistered")"
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)
@@ -1108,7 +1158,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index de7d453..ee4f0ab 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -756,4 +756,16 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '

+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+	git config submodule.example.foo bar &&
+	git submodule deinit &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)"
+'
+
+test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
+	git submodule deinit &&
+	test_must_fail git submodule deinit example
+'
+
 test_done
-- 
1.8.0.1.348.gc64da69

^ permalink raw reply related

* Re: [PATCH v2] t9402: sed -i is not portable
From: Junio C Hamano @ 2012-12-04 20:52 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, mmogilvi_git
In-Reply-To: <201212042044.49477.tboegi@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

> On some systems sed allows the usage of e.g.
> sed -i -e "s/line1/line2/" afile
> to edit the file "in place".
> Other systems don't allow that: one observed behaviour is that
> sed -i -e "s/line1/line2/" afile
> creates a backup file called afile-e, which breaks the test.
> As sed -i is not part of POSIX, avoid it.

Thanks.

The intention is good, but see comments on this part in the patch
below.

> Use test_cmp, makes the test easier to debug.

I see a few other remaining issues in the code after this patch is
applied.  If we are doing other fixes like these, we may want to fix
them as well:

 - test_must_fail is used to run cvs; it shouldn't (instead of
   saying "test_must_fail cvs ...", just say "! cvs ...").

 - A shell function should begin like this: "shellfunc () {"

 - Redirection should not have SP before the filename (i.e. ">out",
   not "> out").

 - It is OK to spell single-liner subshell like this:

    ( cd cvswork3 && ! cvs -f diff -N -u ) &&
    ...

   but a multi-line subshell should start its first command on a
   separate line, like this:

    (
        cd cvswork3 &&
        cvs update foo
    ) &&
    ...

 - quite a few "[cvswork$n]" comments are truncated to "[cvswork$n"
   in the test titles.

> Chain all shell commands with && to detect all kinds of failure.

I think you missed the one in 'merge early [cvswork3] b3 with b1'
where a merge is done in a subshell; a failed merge is not detected.

This is another reason why I asked you not to mix different things
in a single patch; if this patch is rerolled to cover missing "&&"
that you missed, it needs to be re-reviewed for the "sed -i" fix we
review during this round again.

> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> Changes since v1:
> No correction of TABs to make it easier to review
>
> If this is OK:
> Matthew would you like to send a complete re-roll,
> because the credit should be on you ?
>
>  t/t9402-git-cvsserver-refs.sh | 44 ++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
> index 858ef0f..5138f14 100755
> --- a/t/t9402-git-cvsserver-refs.sh
> +++ b/t/t9402-git-cvsserver-refs.sh
> @@ -28,27 +28,26 @@ check_file() {
>  }
>  
>  check_end_tree() {
> -    sandbox="$1"
> -    expectCount=$(wc -l < "$WORKDIR/check.list")
> -    cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
> -    test x"$cvsCount" = x"$expectCount"
> -    stat=$?
> -    echo "check_end $sandbox : $stat cvs=$cvsCount expect=$expectCount" \
> -	>> "$WORKDIR/check.log"
> -    return $stat
> +    sandbox="$1" &&
> +    wc -l < "$WORKDIR/check.list" > expected &&
> +    find "$sandbox" -type f | grep -v "/CVS" > "$WORKDIR/check.cvsCount" &&
> +    wc -l < "$WORKDIR/check.cvsCount" >actual &&

You replaced the original "find" that was perfectly correct with
"pipe to grep -v" which is more expensive.  Why?

The file stores the lines to be counted; why is it called a "Count"?

As "check.list" is really the "expected list of files", it may want
to be renamed to "list.expected" or something like that.  Then the
output from this "find" would naturally become "list.actual", i.e.

	find "$sandbox" -name CVS -type d -prune \
            -o -type f -print >"$WORKDIR/list.actual"

Even though you are comparing only the number of lines in the file
in this function, are they expected to name the same set of paths?
The other function check_end_full_tree used to compare only the
count, but you changed it to compare the actul names of paths, which
may be a more robust and correct test, so it might make more sense
to redefine $WORKDIR/list.{expect,actual} to be a sorted list of
paths relative to the top of the working tree and do something like
this:

	(
        	cd "$sandbox" &&
	        find . -name CVS -type d -prune -o -type f -print
	) | sed -e "s|^./||" | sort >"$WORKDIR/list.actual"

and make sure "$WORKDIR/list.expect" is also sorted.  Then you can
lose the "wc -l" based check from here and compare the list of
paths.

> +    test_cmp expected actual &&
> +		rm expected actual &&
> +		sort < "$WORKDIR/check.list" > expected &&
> +		sort < "$WORKDIR/check.cvsCount" | sed -e "s%cvswork/%%" >actual &&

It is probably easier for a later patch to fix the indentation, if
these new lines followed the existing indentation.

> +    test_cmp expected actual &&
> +		rm expected actual
>  }
>  
>  check_end_full_tree() {
> -    sandbox="$1"
> -    ver="$2"
> -    expectCount=$(wc -l < "$WORKDIR/check.list")
> -    cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
> -    gitCount=$(git ls-tree -r "$2" | wc -l)
> -    test x"$cvsCount" = x"$expectCount" -a x"$gitCount" = x"$expectCount"
> -    stat=$?
> -    echo "check_end $sandbox : $stat cvs=$cvsCount git=$gitCount expect=$expectCount" \
> -	>> "$WORKDIR/check.log"
> -    return $stat
> +    sandbox="$1" &&
> +    sort < "$WORKDIR/check.list" >expected &&
> +    find "$sandbox" -name CVS -prune -o -type f -print | sed -e "s%$sandbox/%%" | sort >act1 &&
> +		test_cmp expected act1 &&
> +    git ls-tree -r "$2" | sed -e "s/^.*blob [0-9a-fA-F]*[	 ]*//" | sort > act2 &&

You can say "git ls-tree --name-only -r" and lose the sed.

^ permalink raw reply

* Git problem with CIFS/SAMBA URLs
From: Nils Fenner @ 2012-12-04 20:14 UTC (permalink / raw)
  To: git

Hi guys,
 
I tried to use url.insteadof to get URLs mapped correctly, starting with a double-slash (//). Unfortunately, Git doesn't recognize that and still tries to connect to the original URL. I am using Git in a productive environment. On Windows desktops, MsysGit 1.7.11 and 1.8.0. On ubuntu clients Git 1.7.9.0 (as delivered with the distro).

Problem is, that we have lots and lots of submodules, with URLs like //thegitserver/.../therepo.git. Now I'd like to checkout on the Ubuntu machine for the CI system. Should not be much of a preblem - well, in theory.

Let's replace the windows url's (//someserver/repo.git) with a mounted CIFS path (/media/samba_share/repo.git). On MsysGit this works as long as the server is reachable. On Ubuntu it completely doesn't work. All comes down to the url-alias, which seems not to be replaced correctly when using windows double-slash url's.
 
I'd like to give some more samples to show what's happening:
 
To access cross-plattform file systems I mounted the windows share via samba (e.g. on /media/samba_share). This works fine and we are able to clone the main repositories by using:
$ git clone /media/samba_share/repo.git
 
Now let's get the submodules:
$ git submodule init -> writes the "wrong" //thegitserver/... URLs in the config.
$ git submodule update -> fails because of the CIFS double-slash url's.

What to do? Simply define a "url.insteadof" alias
[url "/media/samba_share"]
    insteadof = //thegitserver

Uh, oh. Doesn't help.
 
The following would work like expected, but is not usable for my case. I would have to rewrite all URLs, what I don't want to do (maybe in the future):
[url "/media/samba_share"]
  insteadof = file:////serverurl
 
On MsysGit, replacing the double-slash works, as long as the URL is reachable (meaning the server has to be online). Otherwise it behaves the same like Linux.
 
Sample error Message:
fatal: repository '//thegitserver/.../repository.git' does not exist
 
The URL should be: /media/samba_share/.../repository.git
 
This seems to be the only case. Other URLs work like expected.

Seems like this is a bug in Git. Or did I miss something?

Please help!

Cheers,
Nils

^ permalink raw reply

* Re: [PATCH] gitk: read and write a repository specific configuration file
From: Lukasz Stelmach @ 2012-12-04 20:12 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git, paulus, gitster
In-Reply-To: <1kujrnc.1r4khfkkpdoo1M%lists@haller-berlin.de>

W dniu 03.12.2012 22:15, Stefan Haller pisze:
> Lukasz Stelmach <stlman@poczta.fm> wrote:
> 
>> Enable gitk read and write repository specific configuration
>> file: ".git/k" if the file exists. To make gitk use the local
>> file simply create one, e.g. with the touch(1) command.
> 
> I'm not sure I like this proposal. While it may be desirable to have
> *some* settings stored per repository, for most settings I want them to
> be remembered globally.

The way it works with my patch, gitk reads global settings from ~/.gitk.
So you can treat it as a template. Then, when you exit it saves to local
file if it exists. This of course means you can't override settings from
./.git/k with something from ~/.gitk by simply choosing to on GUI.
However, it takes no more than removing appropriate line from .git/k to
get the value from ~/.gitk.

IMHO this is a reasonable compromise which is available at no cost as
far as data structure complexity is concerned. Choosing where to save
what would require a bit of information per configuration variable. With
a mask saved locally surprises may come when you change a variable,
forget to localise it and it pops in a different repository. My
approach, however simplistic, avoids this particular pitfall.


> Git-gui tries to solve this by presenting two panes in the preferences
> dialog, so that I can choose the scope of every setting I change. This
> still doesn't help for things that are remembered implicitly, like the
> window size.
> 
> I don't have good suggestions how to solve this; just pointing out
> problems.
> 
> 


-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP

^ permalink raw reply

* [PATCH v2] t9402: sed -i is not portable
From: Torsten Bögershausen @ 2012-12-04 19:44 UTC (permalink / raw)
  To: git, mmogilvi_git; +Cc: tboegi

On some systems sed allows the usage of e.g.
sed -i -e "s/line1/line2/" afile
to edit the file "in place".
Other systems don't allow that: one observed behaviour is that
sed -i -e "s/line1/line2/" afile
creates a backup file called afile-e, which breaks the test.
As sed -i is not part of POSIX, avoid it.

Use test_cmp, makes the test easier to debug.
Chain all shell commands with && to detect all kinds of failure.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Changes since v1:
No correction of TABs to make it easier to review

If this is OK:
Matthew would you like to send a complete re-roll,
because the credit should be on you ?

 t/t9402-git-cvsserver-refs.sh | 44 ++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
index 858ef0f..5138f14 100755
--- a/t/t9402-git-cvsserver-refs.sh
+++ b/t/t9402-git-cvsserver-refs.sh
@@ -28,27 +28,26 @@ check_file() {
 }
 
 check_end_tree() {
-    sandbox="$1"
-    expectCount=$(wc -l < "$WORKDIR/check.list")
-    cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
-    test x"$cvsCount" = x"$expectCount"
-    stat=$?
-    echo "check_end $sandbox : $stat cvs=$cvsCount expect=$expectCount" \
-	>> "$WORKDIR/check.log"
-    return $stat
+    sandbox="$1" &&
+    wc -l < "$WORKDIR/check.list" > expected &&
+    find "$sandbox" -type f | grep -v "/CVS" > "$WORKDIR/check.cvsCount" &&
+    wc -l < "$WORKDIR/check.cvsCount" >actual &&
+    test_cmp expected actual &&
+		rm expected actual &&
+		sort < "$WORKDIR/check.list" > expected &&
+		sort < "$WORKDIR/check.cvsCount" | sed -e "s%cvswork/%%" >actual &&
+    test_cmp expected actual &&
+		rm expected actual
 }
 
 check_end_full_tree() {
-    sandbox="$1"
-    ver="$2"
-    expectCount=$(wc -l < "$WORKDIR/check.list")
-    cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
-    gitCount=$(git ls-tree -r "$2" | wc -l)
-    test x"$cvsCount" = x"$expectCount" -a x"$gitCount" = x"$expectCount"
-    stat=$?
-    echo "check_end $sandbox : $stat cvs=$cvsCount git=$gitCount expect=$expectCount" \
-	>> "$WORKDIR/check.log"
-    return $stat
+    sandbox="$1" &&
+    sort < "$WORKDIR/check.list" >expected &&
+    find "$sandbox" -name CVS -prune -o -type f -print | sed -e "s%$sandbox/%%" | sort >act1 &&
+		test_cmp expected act1 &&
+    git ls-tree -r "$2" | sed -e "s/^.*blob [0-9a-fA-F]*[	 ]*//" | sort > act2 &&
+		test_cmp expected act2 &&
+    rm expected act1 act2
 }
 
 #########
@@ -155,7 +154,8 @@ test_expect_success 'cvs co b1 [cvswork3]' '
 
 test_expect_success 'edit cvswork3 and save diff' '
     ( cd cvswork3 &&
-      sed -i -e "s/line1/line1 - data/" adir/afile &&
+      sed -e "s/line1/line1 - data/" adir/afile >adir/afileNEW &&
+			mv -f adir/afileNEW adir/afile &&
       echo "afile5" > adir/afile5 &&
       rm t2 &&
       cvs -f add adir/afile5 &&
@@ -168,7 +168,8 @@ test_expect_success 'setup v1.2 on b1' '
     git checkout b1 &&
     echo "new v1.2" > t3 &&
     rm t2 &&
-    sed -i -e "s/line3/line3 - more data/" adir/afile &&
+    sed -e "s/line3/line3 - more data/" adir/afile >adir/afileNEW &&
+		mv -f adir/afileNEW adir/afile &&
     rm adir/a2file &&
     echo "a3file" >> adir/a3file &&
     echo "bfile line 3" >> adir/bdir/bfile &&
@@ -300,7 +301,8 @@ test_expect_success 'root dir rm file [cvswork2]' '
 
 test_expect_success 'subdir edit/add/rm files [cvswork2' '
     ( cd cvswork2 &&
-      sed -i -e "s/line 1/line 1 (v2)/" adir/bdir/bfile &&
+      sed -e "s/line 1/line 1 (v2)/" adir/bdir/bfile >adir/bdir/bfileNEW &&
+      mv -f adir/bdir/bfileNEW adir/bdir/bfile &&
       rm adir/bdir/b2file &&
       cd adir &&
       cvs -f rm bdir/b2file &&
-- 
1.8.0.197.g5a90748

^ permalink raw reply related

* Re: [RFC] Add basic syntax check on shell scripts
From: Junio C Hamano @ 2012-12-04 19:39 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Torsten Bögershausen, git
In-Reply-To: <CACsJy8BxviWRHqGvptsJVmkFM6HQa9HnLWsh5V6Ec6Fqv52sGA@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

>> This (once it gets cleaned up to reduce false positives) belongs to
>> "cd t && make test-lint".
>
> Or a project commit hook?

Surely.  It is OK to have "cd t && make test-lint" in your
pre-commit hook.

A few more things in addition to what Torsten's script attempts to
catch that we would want to catch are:

 * Do not spell string equality with "test $a == $b"; that is
   bash-ism and you only need "=" (which works in bash, too);

 * Do not capture output from "wc -l" in a variable and string
   compare with a constant, e.g. 

	lnum=$(wc -l <...) && test "$lnum" = 9

   as some wc implementations place extra SP in its output;

 * Do not use "test_must_fail" to run non-git command and require it
   to fail (instead, just write "! cmd").

 * Do not write ERE with backslashes and expect "grep" to grok them;
   that's GNUism.  e.g.

	grep "^\(author\|committer\) "

   is bad.  Use egrep (or "grep -E") if you want to use ERE.

^ permalink raw reply

* Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
From: Erik Faye-Lund @ 2012-12-04 17:40 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit, peff
In-Reply-To: <alpine.DEB.1.00.1212041809150.31987@s15462909.onlinehome-server.info>

On Tue, Dec 4, 2012 at 6:12 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Sat, 1 Dec 2012, Erik Faye-Lund wrote:
>
>> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
>> >
>> >> Set a control-handler to prevent the process from terminating, and
>> >> simulate SIGINT so it can be handled by a signal-handler as usual.
>> >
>> > One thing you might want to mention is that the fgetc() handling is not
>> > thread-safe, and intentionally so: if two threads read from the same
>> > console, we are in trouble anyway.
>>
>> I'm not entirely sure if I know what you mean. Do you suggest that two
>> threads can race for setting the console ctrl-handler?
>
> That was my idea, yes.
>
>> I don't think that's the case; "SetConsoleCtrlHandler(x, TRUE)" adds a
>> console handler to the handler-chain, and SetConsoleCtrlHandler(x,
>> FALSE) removes it. If two threads add handlers, it is my understanding
>> that one of them will be run, only to report "no, no more ctrl-handling
>> needed". Since both handlers block further ctrl-handling, I don't think
>> there's a problem.
>
> My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the
> handler prematurely iff another thread wanted to install the very same
> handler (but it was already installed by the first thread).
>

Yes, but that's not how SetConsoleCtrlHandler works; if a routine x
gets added twice, it needs to be removed twice as well to not get
called. This little C-program demonstrates that:

---8<---
#include <windows.h>
#include <stdio.h>

BOOL WINAPI HandlerRoutine1(DWORD dwCtrlType)
{
        printf("Hello from handler1!\n");
        return TRUE;
}

BOOL WINAPI HandlerRoutine2(DWORD dwCtrlType)
{
        printf("Hello from handler2!\n");
        return FALSE;
}

int main()
{
        SetConsoleCtrlHandler(HandlerRoutine1, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
        GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0);
        SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
        SetConsoleCtrlHandler(HandlerRoutine1, FALSE);
}
---8<---

This program outputs:
Hello from handler2!
Hello from handler1!

So since that other thread would add the console ctrl handler before
it removed it again, this should still be thread-safe as far as I can
tell.

> But as I said: for this to happen, *two* threads need to want to access
> the console for reading. In that case we'd be in bigger trouble than
> thread unsafety... We cannot read two passwords from the same console at
> the same time.

I agree with that, but I'm saying that *even if* we didn't have this
limitation, the code shouldn't be problematic.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* git-svn with non-standard repository layout
From: Piotr Krukowiecki @ 2012-12-04 17:29 UTC (permalink / raw)
  To: Git Mailing List

Hi,

Is there a way to handle svn repository with following layout?

repo/trunk
repo/branches/branch1
repo/branches/branch2
repo/branches/work/developer1/branch3
repo/branches/work/developer1/branch4
repo/branches/work/developer2/branch5

In default configuration it treats "work" dir as a branch. If I
configure it with branches = branches/work/*/* it recognizes them
correctly, but then the "normal" branch1, branch2 are lost.

Ideally, I'd have one git repo with trunk, branches/{branch1,branch2},
work/developerX/branchY branches...

Any ideas?

--
Piotr Krukowiecki

^ permalink raw reply

* Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling
From: Johannes Schindelin @ 2012-12-04 17:12 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, msysgit, peff
In-Reply-To: <CABPQNSY_vHdvBvU_ezjyOzoZeBJAYTJ2829o6Vxs-nJjQVcvDQ@mail.gmail.com>

Hi kusma,

On Sat, 1 Dec 2012, Erik Faye-Lund wrote:

> On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
> >
> >> By moving the echo-disabling code to a separate function, we can
> >> implement OS-specific versions of it for non-POSIX platforms.
> >>
> >> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> >> ---
> >>  compat/terminal.c | 43 +++++++++++++++++++++++++------------------
> >>  1 file changed, 25 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/compat/terminal.c b/compat/terminal.c
> >> index bbb038d..3217838 100644
> >> --- a/compat/terminal.c
> >> +++ b/compat/terminal.c
> >> @@ -14,6 +14,7 @@ static void restore_term(void)
> >>               return;
> >>
> >>       tcsetattr(term_fd, TCSAFLUSH, &old_term);
> >> +     close(term_fd);
> >>       term_fd = -1;
> >>  }
> >
> > That looks like an independent resource leak fix... correct?
> 
> It might look like it, but it's not; term_fd used to be returned by
> "fileno(fh)", and fh did get properly closed.
> 
> With my refactoring, disable_echo/restore_term takes opens /dev/tty a
> second time, like Jeff points out. And that second file descriptor
> needs to be closed.

Thanks for clarifying,
Dscho

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
From: Johannes Schindelin @ 2012-12-04 17:12 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, msysgit, peff
In-Reply-To: <CABPQNSaCV820zhJGdW++LMf2U7AeODSbfukix3fMfffmNex4YA@mail.gmail.com>

Hi kusma,

On Sat, 1 Dec 2012, Erik Faye-Lund wrote:

> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
> >
> >> Set a control-handler to prevent the process from terminating, and
> >> simulate SIGINT so it can be handled by a signal-handler as usual.
> >
> > One thing you might want to mention is that the fgetc() handling is not
> > thread-safe, and intentionally so: if two threads read from the same
> > console, we are in trouble anyway.
> 
> I'm not entirely sure if I know what you mean. Do you suggest that two
> threads can race for setting the console ctrl-handler?

That was my idea, yes.

> I don't think that's the case; "SetConsoleCtrlHandler(x, TRUE)" adds a
> console handler to the handler-chain, and SetConsoleCtrlHandler(x,
> FALSE) removes it. If two threads add handlers, it is my understanding
> that one of them will be run, only to report "no, no more ctrl-handling
> needed". Since both handlers block further ctrl-handling, I don't think
> there's a problem.

My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the
handler prematurely iff another thread wanted to install the very same
handler (but it was already installed by the first thread).

But as I said: for this to happen, *two* threads need to want to access
the console for reading. In that case we'd be in bigger trouble than
thread unsafety... We cannot read two passwords from the same console at
the same time.

Ciao,
Dscho

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply

* Re: [PATCH] mingw_rmdir: do not prompt for retry when non-empty
From: Johannes Schindelin @ 2012-12-04 16:35 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, msysgit
In-Reply-To: <1354617713-7436-1-git-send-email-kusmabite@gmail.com>

Hi kusma,

On Tue, 4 Dec 2012, Erik Faye-Lund wrote:

> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
> a check was added to prevent us from retrying to delete a directory
> that is both in use and non-empty.
> 
> However, this logic was slightly flawed; since we didn't return
> immediately, we end up falling out of the retry-loop, but right into
> the prompting loop.
> 
> Fix this by simply returning from the function instead of breaking
> the loop.
> 
> While we're at it, change the second break to a return as well; we
> already know that we won't enter the prompting-loop, beacuse
> is_file_in_use_error(GetLastError()) already evaluated to false.

I usually prefer to break from the loop, to be able to add whatever
cleanup code we might need in the future after the loop.

So does this fix the problem for you?

-- snipsnap --
diff --git a/compat/mingw.c b/compat/mingw.c
index 04af3dc..504495a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -259,7 +259,8 @@ int mingw_rmdir(const char *pathname)
 		return -1;
 
 	while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
-		if (!is_file_in_use_error(GetLastError()))
+		errno = err_win_to_posix(GetLastError());
+		if (errno != EACCESS)
 			break;
 		if (!is_dir_empty(wpathname)) {
 			errno = ENOTEMPTY;
@@ -275,7 +276,7 @@ int mingw_rmdir(const char *pathname)
 		Sleep(delay[tries]);
 		tries++;
 	}
-	while (ret == -1 && is_file_in_use_error(GetLastError()) &&
+	while (ret == -1 && errno == EACCESS &&
 	       ask_yes_no_if_possible("Deletion of directory '%s' failed. "
 			"Should I try again?", pathname))
 	       ret = _wrmdir(wpathname);

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related

* Re: Python extension commands in git - request for policy change
From: Martin Langhoff @ 2012-12-04 15:51 UTC (permalink / raw)
  To: Eric S. Raymond; +Cc: Git Mailing List
In-Reply-To: <20121125024451.1ADD14065F@snark.thyrsus.com>

On Sat, Nov 24, 2012 at 9:44 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> git presently contains one Python extension command, Pete Wycoff's p4
> importer.  If my git-weave code is merged it will acquire another.

Write a really compelling tool. Don't argue languages. Make it
wonderful. The git maintainers, tool maintainers (you!) and overall
development community have shown flexibility and smarts in the past.

cheers,


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

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Stephen Bash @ 2012-12-04 14:40 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: esr, Magnus Bäck, Michael Haggerty, git, Philippe Vaucher
In-Reply-To: <CAMP44s0rcy6OfMPM+8BhQy0DbxRLBHEsraHw0u4oAZzh5euTzg@mail.gmail.com>

----- Original Message -----
> From: "Felipe Contreras" <felipe.contreras@gmail.com>
> Sent: Tuesday, December 4, 2012 9:19:18 AM
> Subject: Re: Python extension commands in git - request for policy change
> 
> > > Also, you are ignoring all the advantages that shell has and
> > > python does not.
> >
> > Out of curiosity, can you list the advantages? From what I
> > gathered:
> >
> > - no need to install bash
> 
> Unless you are in Windows or OS X. OS X has a shell, but not bash.

>From http://support.apple.com/kb/TA27005:

"The default shell (or command-line interface) used in Mac OS X 10.0 through 10.2.8 is tcsh (with 10.3 and 10.4 it's bash). With Mac OS X 10.2 or later, other interactive shells are included, such as bash and zsh."

Stephen

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-12-04 14:19 UTC (permalink / raw)
  To: Philippe Vaucher
  Cc: esr, Magnus Bäck, Michael Haggerty, git@vger.kernel.org
In-Reply-To: <CAGK7Mr4HkCkbw-SV-d=JAmQieV0ZQOE7YqR-g7rTWzHGPYqzHA@mail.gmail.com>

On Mon, Dec 3, 2012 at 3:45 PM, Philippe Vaucher
<philippe.vaucher@gmail.com> wrote:
>> Also, you are ignoring all the advantages that shell has and python does not.
>
> Out of curiosity, can you list the advantages? From what I gathered:
>
> - no need to install bash

Unless you are in Windows or OS X. OS X has a shell, but not bash.

> - git contributors are more used to bash

I don't see this as a big advantage.

> - there's only one "version" of bash (no real need to handle different
> versions compared to py26, py27, etc)

This one is.

The language doesn't change much from different versions of bash either way.

- the language is part of POSIX

Which means you don't need bash, you can use dash, or zsh, or many other shells.

- the language is simple

It's so simple it's used in shells.

- no need for extensions

All you need is the shell binary, that's it.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [ANNOUNCE] Git v1.8.1-rc0
From: Michael J Gruber @ 2012-12-04 14:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vip8iq1vs.fsf@alter.siamese.dyndns.org>

While not being a huge problem, you may or may not want to correct one
commit message:

*   aaf5ad5 (origin/next, gitster/next) Sync with 1.8.0-rc0
|\
| * ee26a6e (tag: v1.8.1-rc0, origin/master, origin/HEAD,
gitster/master) Git 1.8.1-rc0

Cheers,
Michael

^ permalink raw reply

* Re: [PATCH v7 p1 07/13] remote-testgit: remove irrelevant test
From: Felipe Contreras @ 2012-12-04 14:03 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <20121203231321.GA5098@padd.com>

On Mon, Dec 3, 2012 at 5:13 PM, Pete Wyckoff <pw@padd.com> wrote:

>> --- a/t/t5801-remote-helpers.sh
>> +++ b/t/t5801-remote-helpers.sh
>> @@ -53,19 +53,6 @@ test_expect_success 'pushing to local repo' '
>>       compare_refs localclone HEAD server HEAD
>>  '
>>
>> -# Generally, skip this test.  It demonstrates a now-fixed race in
>> -# git-remote-testgit, but is too slow to leave in for general use.
>> -: test_expect_success 'racily pushing to local repo' '
>> -     test_when_finished "rm -rf server2 localclone2" &&
>> -     cp -R server server2 &&
>> -     git clone "testgit::${PWD}/server2" localclone2 &&
>> -     (cd localclone2 &&
>> -     echo content >>file &&
>> -     git commit -a -m three &&
>> -     GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
>> -     compare_refs localclone2 HEAD server2 HEAD
>> -'
>> -
>
> Why does this cause problems?

It doesn't.

> If you're going to rip it out, please finish the job, and take
> out the other two hunks that are needed to trigger this test.  See
> 7fb8e16 (git-remote-testgit: fix race when spawning fast-import,
> 2012-04-22).

That commit has absolutely nothing to do with the new remote-testgit,
the old remote-testgit is called now remote-testpy, and its test has
this chunk still there.

For the new remote-testgit this is irrelevant.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH v2 0/4] git-svn: More docs for branch handling in
From: Eric Wong @ 2012-12-04 11:39 UTC (permalink / raw)
  To: Sebastian Leske; +Cc: git, Junio C Hamano, Michael J Gruber
In-Reply-To: <7vvccjorsf.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> I've sent comments on small nits I found but overall they looked
> quite well researched.  Will tentatively queue on the 'pu' branch,
> expecting further updates and Acks from people involved polishing
> these patches.
> 
> Thanks for writing it up.

Thanks all.  I think the patches are good, but also look forward
to improvements following Junio's comments.

^ permalink raw reply

* [PATCH] mingw_rmdir: do not prompt for retry when non-empty
From: Erik Faye-Lund @ 2012-12-04 10:41 UTC (permalink / raw)
  To: git; +Cc: msysgit, johannes.schindelin

in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
a check was added to prevent us from retrying to delete a directory
that is both in use and non-empty.

However, this logic was slightly flawed; since we didn't return
immediately, we end up falling out of the retry-loop, but right into
the prompting loop.

Fix this by simply returning from the function instead of breaking
the loop.

While we're at it, change the second break to a return as well; we
already know that we won't enter the prompting-loop, beacuse
is_file_in_use_error(GetLastError()) already evaluated to false.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

Here's a quick patch for a small issue I recently encountered; when
deleting a file from inside a directory, we currently end up
prompting the user if (s)he want us to retry deleting the directory
they are in.

 compat/mingw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 1eb974f..2c29667 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -260,10 +260,10 @@ int mingw_rmdir(const char *pathname)
 
 	while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
 		if (!is_file_in_use_error(GetLastError()))
-			break;
+			return ret;
 		if (!is_dir_empty(wpathname)) {
 			errno = ENOTEMPTY;
-			break;
+			return ret;
 		}
 		/*
 		 * We assume that some other process had the source or
-- 
1.8.0.msysgit.0.3.g0262b9f.dirty

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related

* Re: [PATCH] fsck: warn about ".git" in trees
From: Andreas Ericsson @ 2012-12-04 10:40 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <50B90E11.8090501@web.de>

On 11/30/2012 08:50 PM, Torsten Bögershausen wrote:
>> Having a ".git" entry inside a tree can cause confusing
>> results on checkout. At the top-level, you could not
>> checkout such a tree, as it would complain about overwriting
>> the real ".git" directory. In a subdirectory, you might
>> check it out, but performing operations in the subdirectory
>> would confusingly consider the in-tree ".git" directory as
>> the repository.
> [snip]
>> +    int has_dotgit = 0;
> 
> Name like "." or ".." are handled as directories by the OS.
> 

The patch is for the index, where they're handled as whatever the mode
claims it is. The patch doesn't touch those parts though.

> ".git" could be a file or a directory, at least in theory,
> and from the OS point of view,
> but we want to have this as a reserved name.
> 
> Looking at bad directory names, which gives trouble when checking out:
> 
> Should we check for "/" or "../blabla" as well?
> 

Apart from the checks already in place, checking for git's internal
directory separator marker (which is '/') is enough to catch both,
and that check is done.

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

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

^ permalink raw reply

* Re: [PATCH] status: respect advice.statusHints for ahead/behind advice
From: Matthieu Moy @ 2012-12-04  9:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vr4n7rrgb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> So let's do a lot simpler patch instead.
>
> -- >8 --
> From: Jeff King <peff@peff.net>
> Date: Mon, 3 Dec 2012 01:16:57 -0500
>
> If the user has unset advice.statusHints, we already
> suppress the "use git reset to..." hints in each stanza. The
> new "use git push to publish..." hint is the same type of
> hint. Let's respect statusHints for it, rather than making
> the user set yet another advice flag.

You may want to squash this on top:

-- >8 --

>From 87a438bd06dd689bd37949e762690ab5dbfc5ff8 Mon Sep 17 00:00:00 2001
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Tue, 4 Dec 2012 10:15:03 +0100
Subject: [PATCH] document that statusHints affects git checkout

---
 Documentation/config.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e70216d..bf8f911 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -160,9 +160,10 @@ advice.*::
                it resulted in a non-fast-forward error.
        statusHints::
                Show directions on how to proceed from the current
-               state in the output of linkgit:git-status[1] and in
+               state in the output of linkgit:git-status[1], in
                the template shown when writing commit messages in
-               linkgit:git-commit[1].
+               linkgit:git-commit[1], and in the help message shown
+               by linkgit:git-checkout[1] when switching branch.
        commitBeforeMerge::
                Advice shown when linkgit:git-merge[1] refuses to
                merge to avoid overwriting local changes.
-- 
1.8.0.319.g8abfee4



-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply related

* [PATCH v2 6/6] mingw: get rid of getpass implementation
From: Erik Faye-Lund @ 2012-12-04  8:10 UTC (permalink / raw)
  To: git; +Cc: msysgit, johannes.schindelin, gitster, peff
In-Reply-To: <1354608642-5316-1-git-send-email-kusmabite@gmail.com>

There's no remaining call-sites, and as pointed out in the
previous commit message, it's not quite ideal. So let's just
lose it.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c | 15 ---------------
 compat/mingw.h |  2 --
 2 files changed, 17 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 33ddfdf..5fc14b7 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1758,21 +1758,6 @@ int link(const char *oldpath, const char *newpath)
 	return 0;
 }
 
-char *getpass(const char *prompt)
-{
-	struct strbuf buf = STRBUF_INIT;
-
-	fputs(prompt, stderr);
-	for (;;) {
-		char c = _getch();
-		if (c == '\r' || c == '\n')
-			break;
-		strbuf_addch(&buf, c);
-	}
-	fputs("\n", stderr);
-	return strbuf_detach(&buf, NULL);
-}
-
 pid_t waitpid(pid_t pid, int *status, int options)
 {
 	HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
diff --git a/compat/mingw.h b/compat/mingw.h
index 6b9e69a..f494ecb 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -55,8 +55,6 @@ struct passwd {
 	char *pw_dir;
 };
 
-extern char *getpass(const char *prompt);
-
 typedef void (__cdecl *sig_handler_t)(int);
 struct sigaction {
 	sig_handler_t sa_handler;
-- 
1.8.0.4.g3c6fb4f.dirty

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

^ permalink raw reply related


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