Re: [PATCH] checkout: add --sparse for restoring files in sparse checkout mode

2013-03-24 Thread Eric Sunshine
On Sun, Mar 24, 2013 at 1:06 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
 index 8edcdca..45a2b53 100644
 --- a/Documentation/git-checkout.txt
 +++ b/Documentation/git-checkout.txt
 @@ -180,6 +180,13 @@ branch by running git rm -rf . from the top level of 
 the working tree.
 +--sparse::
 +   In sparse checkout mode, `git checkout -- paths` would
 +   update all entries matched by paths regardless sparse

s/regardless/regardless of/

 +   patterns. This option only updates entries matched by paths
 +   and sparse patterns.
 +
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] checkout: avoid unncessary match_pathspec calls

2013-03-24 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 ---
  Junio, this patch clearly conflicts wih nd/magic-pathspecs. Do you
  want me to:

   - hold it off until nd/magic-pathspecs graduates
   - rebase on top of nd/magic-pathspecs and repost
   - leave it to you to handle conflicts
  ?

I'd prefer to take small, independent and clear improvements first
and worry about larger ones later, so if there were another choice,
i.e.

 - eject nd/magic-pathspecs for now, cook this (and other small
   independent and clear improvements you may come up with, some of
   which might come out of nd/magic-pathspecs itself) and let it
   graduate first, and later revisit rerolld nd/magic-pathspecs

that would be the ideal among the given choices ;-).

   for (pos = 0; pos  active_nr; pos++) {
   struct cache_entry *ce = active_cache[pos];
 + ce-ce_flags = ~CE_MATCHED;
   if (opts-source_tree  !(ce-ce_flags  CE_UPDATE))
   continue;
 - match_pathspec(opts-pathspec, ce-name, ce_namelen(ce), 0, 
 ps_matched);
 + if (match_pathspec(opts-pathspec, ce-name,
 +ce_namelen(ce), 0, ps_matched))
 + ce-ce_flags |= CE_MATCHED;
   }
  
   if (report_path_error(ps_matched, opts-pathspec, opts-prefix))
   return 1;
  
 + /*
 +  * call match_pathspec on the remaining entries that have not
 +  * been done in the previous loop
 +  */
 + for (pos = 0; pos  active_nr; pos++) {
 + struct cache_entry *ce = active_cache[pos];
 + if (opts-source_tree  !(ce-ce_flags  CE_UPDATE) 
 + match_pathspec(opts-pathspec, ce-name,
 +ce_namelen(ce), 0, ps_matched))
 + ce-ce_flags |= CE_MATCHED;
 + }
 +

The above is a faithful rewrite, but I have to wonder why you need
two separate loops.

Do you understand what the original loop is doing with ps_matched,
and why the code excludes certain paths while doing so?  I didn't
when I read your patch for the first time, as I forgot, until I
checked with 0a1283bc3955 (checkout $tree $path: do not clobber
local changes in $path not in $tree, 2011-09-30)

You don't use ps_matched after report_path_error(); the new loop
shouldn't have to record which pathspec matched.

Also I notice that I forgot to free ps_matched.  Perhaps doing it
this way is easier to maintain?

/*
 * Make sure all pathspecs participated in locating the
 * paths to be checked out.
 */
for (pos = 0; pos  active_nr; pos++) {
if (opts-source_tree  !(ce-ce_flags  CE_UPDATE))
/*
 * git checkout tree-ish -- path, but this entry
 * is in the original index; it will not be checked
 * out to the working tree and it does not matter
 * if pathspec matched this entry.  We will not do
 * anything to this entry at all.
 */
verify_psmatch = NULL;
else
/*
 * Either this entry came from the tree-ish
 * we are checking the paths out of, or we
 * are checking out of the index.
 */
verify_psmatch = ps_matched;
if (match_pathspec(opts-pathspec, ce-name, ce_namelen(ce),
   0, verify_psmatch))
ce-ce_flags |= CE_MATCHED;
}

if (report_path_error(ps_matched, opts-pathspec, opts-prefix))
return 1;
free(ps_matched);

After commenting on the above, it makes me wonder if we even need to
bother marking entries that were in the index that did not come from
the tree-ish we are checking paths out of, though.  What breaks if
you did not do the rewrite above and dropped the second loop in your
patch?


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] refs.c: fix fread error handling

2013-03-24 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: feature request - have git honor nested .gitconfig files

2013-03-24 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Yeah, I'm not planning to work on this, but I'd be happy to review
 patches if somebody else wants to.

I am not planning to work on this, and honestly speaking I would not
be very happy to see any patch in this area.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] drop some obsolete x = x compiler warning hacks

2013-03-24 Thread Torsten Bögershausen
On 21.03.13 21:47, Jonathan Nieder wrote:
 Jeff King wrote:
 
 And 4.3 was old enough for me to say I do not care if you can run with
 -Wall -Werror or not, let alone 4.2.
 
 Changes like this can only reveal bugs (in git or optimizers) that
 were hidden before, without regressing actual runtime behavior, so for
 what it's worth I like them.
 
 I think perhaps we should encourage people to use
 -Wno-error=uninitialized, in addition to cleaning up our code where
 reasonably recent optimizers reveal it to be confusing.
 
 Reviewed-by: Jonathan Nieder jrnie...@gmail.com

I got 2 warnings, but reading the comments I feel that

Mac OS 10.6 and i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 
5666) (dot 3)

is outdated ;-)


builtin/cat-file.c: In function ~cmd_cat_file~:
builtin/cat-file.c:196: warning: ~contents~ may be used uninitialized in this 
function
builtin/cat-file.c:196: note: ~contents~ was declared here


fast-import.c: In function ‘parse_new_commit’:
fast-import.c:2438: warning: ‘oe’ may be used uninitialized in this function
fast-import.c:2438: note: ‘oe’ was declared here

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/5] Speed up log -L... -M

2013-03-24 Thread Eric Sunshine
On Sat, Mar 23, 2013 at 5:04 AM, Jeff King p...@peff.net wrote:
 On Sat, Mar 23, 2013 at 06:58:48AM +0100, Thomas Rast wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:
  On Thu, Mar 21, 2013 at 8:52 AM, Thomas Rast tr...@student.ethz.ch wrote:
  This is a bit hacky and should really be replaced by equivalent
  support in --follow, and just using that.  However, in the meantime it
 
  s/using/use/

 I'm not a native speaker, but I really think 'using' is more correct
 here.

 Cannot...resist...grammar discussion.

 I think you are both potentially right.

 You might consider the two items equivalent support and using that
 to be two noun phrases that are objects of the preposition by, and
 that the writer simply omits the second by after the and. In which
 case you are making a noun phrase from a verb phrase, and would want to
 use the gerund form using.  And the sentence, simplifying out some
 modifiers and adding the missing by (which is fine to omit, but the
 parts of speech become much clearer with it there), looks like:

   ...should be replaced by equivalent support, and by using that.

 However, you could also argue that the final clause is a second verb
 phrase for this should which just omits the extra should (which is
 also OK in a list. In which case use acts as a verb, and parses as:

   ...should be replaced by equivalent support, and this should just use
   that.

 So I think it is correct either way, and though it parses slightly
 differently, the overall meaning is the same.

 Phew. Totally not worth that much discussion, but for some reason I find
 these sorts of ambiguous language cases interesting.

Wishing to avoid bike-shedding the commit message, I suggested
s/using/use/ as a minor change to help clarify the grammar a bit.
However, perhaps it could be rephrased as:

  This is a bit hacky and should really be replaced by equivalent
  support in --follow, which can then be employed instead.

-- ES
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

2013-03-24 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

You should always trust but verify what other commands do at key
points of the operation; and I think this require-clean-work-tree
is a key precondition for this mode of operation to work correctly.

Especially because you do not even bother to check the result of
git stash before continuing ;-).

 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

 Delaying to run git stash as much as possible is fine, but with
 the above, can the user tell if something was stashed and she has
 to stash pop once she is done helping the command to resolve the
 conflicts, or she shouldn't run stash pop after she is done
 (because if nothing is stashed at this point, that pop will pop an
 unrelated stash)?

 I could easily tell, from the git pull output: if conflict, then
 stash hasn't been applied.

The question was about git stash save.  Depending on the cleanness
of the tree when git pull was started, save may or may not have
been done.  After resolving a conflicted git pull and committing
the result, the user does not have much clue if she needs to pop
anything, does she?  Instead of the usual resolve the conflicts and
commit the result, she may need to see resolve the conflicts,
commit the result, *AND* UNSTASH.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 8/4] match-trees: drop x = x initializations

2013-03-24 Thread Jeff King
On Sat, Mar 23, 2013 at 09:55:53PM -0700, Junio C Hamano wrote:

 René Scharfe rene.scha...@lsrfire.ath.cx writes:
 
  Hmm, let's see if we can help the compiler follow the code without
  making it harder for people to understand.  The patch looks a bit
  jumbled, but the resulting code is OK in my biased opinion.
 
 I actually think the result is much better than a mere OK; the
 duplicated at this point we know path1 (or path2) is missing from
 the other side has been bothering me and I was about to suggest a
 similar rewrite before I read your message ;-)
 
 However, the same compiler still thinks {elem,path,mode}1 can be
 used uninitialized (but not {elem,path,mode}2).  The craziness I
 reported in the previous message is also the same.  With this patch
 on top to swap the side we inspect first, the compiler thinks
 {elem,path,mode}2 can be used uninitialized but not the other three
 variables X-.

Yeah, I'd agree that the result is more readable, but it does not
address the original problem. Junio, do you want to drop my patch,
squash the initialization of mode into René's version, and just add a
note to the commit message that we still have to deal with the gcc
warning?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] user-manual: Fix the interactive rebase example commit range

2013-03-24 Thread W. Trevor King
From: W. Trevor King wk...@tremily.us

6c26bf4 (user-manual: Reorganize the reroll sections, adding 'git
rebase -i', 2013-02-19) used deadbee as the oldest commit in the pick
list, but as the final commit in the rebased range, due to sloppy
duplication and extension from git-rebase.txt.  This fixes that by
adding a new HEAD commit.

I also reworded the example commit summaries (onelines), because I
think my initial mistake may have been to to misinterpreting this
commit as HEAD without thinking things through ;).

Signed-off-by: W. Trevor King wk...@tremily.us
---
 Documentation/user-manual.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 5f36f81..a839e57 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -2617,11 +2617,12 @@ This will open your editor with a list of steps to be 
taken to perform
 your rebase.
 
 -
-pick deadbee The oneline of this commit
-pick fa1afe1 The oneline of the next commit
+pick deadbee The oneline of HEAD~4
+pick fa1afe1 The oneline of HEAD~3
 ...
+pick 1cef15h The oneline of HEAD
 
-# Rebase c0ffeee..deadbee onto c0ffeee
+# Rebase c0ffeee..1cef15h onto c0ffeee
 #
 # Commands:
 #  p, pick = use commit
-- 
1.8.2.rc0.16.g20a599e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/3] git-merge-one-file error reporting

2013-03-24 Thread Kevin Bracey
Style clean up, as requested, followed by the fix to the both sides added
handling for git-merge-one-file.

This is based on v4 of my p4merge series, as they touch the same area.

Kevin Bracey (3):
  git-merge-one-file: style cleanup
  git-merge-one-file: send ERROR: messages to stderr
  git-merge-one-file: revise merge error reporting

 git-merge-one-file.sh | 50 +-
 1 file changed, 25 insertions(+), 25 deletions(-)

-- 
1.8.2.rc3.21.g744ac65

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/2] mergetools/p4merge: swap LOCAL and REMOTE

2013-03-24 Thread Kevin Bracey
Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that
the incoming branch is now in the left-hand, blue triangle pane, and the
current branch is in the right-hand, green circle pane.

This change makes use of P4Merge consistent with its built-in help, its
reference documentation, and Perforce itself. But most importantly, it
makes merge results clearer. P4Merge is not totally symmetrical between
left and right; despite changing a few text labels from theirs/ours to
left/right when invoked manually, it still retains its original
Perforce theirs/ours viewpoint.

Most obviously, in the result pane P4Merge shows changes that are common
to both branches in green. This is on the basis of the current branch
being green, as it is when invoked from Perforce; it means that lines in
the result are blue if and only if they are being changed by the merge,
making the resulting diff clearer.

Note that P4Merge now shows ours on the right for both diff and merge,
unlike other diff/mergetools, which always have REMOTE on the right.
But observe that REMOTE is the working tree (ie ours) for a diff,
while it's another branch (ie theirs) for a merge.

Ours and theirs are reversed for a rebase - see git help rebase.
However, this does produce the desired show the results of this commit
effect in P4Merge - changes that remain in the rebased commit (in your
branch, but not in the new base) appear in blue; changes that do not
appear in the rebased commit (from the new base, or common to both) are
in green. If Perforce had rebase, they'd probably not swap ours/theirs,
but make P4Merge show common changes in blue, picking out our changes in
green. We can't do that, so this is next best.

Signed-off-by: Kevin Bracey ke...@bracey.fi
Reviewed-by: David Aguilar dav...@gmail.com
---
No change to part 1/2 from previous version, apart from Reviewed-by.
Part 2/2 is modified.

 mergetools/p4merge | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mergetools/p4merge b/mergetools/p4merge
index 8a36916..46b3a5a 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -22,7 +22,7 @@ diff_cmd () {
 merge_cmd () {
touch $BACKUP
$base_present || $BASE
-   $merge_tool_path $BASE $LOCAL $REMOTE $MERGED
+   $merge_tool_path $BASE $REMOTE $LOCAL $MERGED
check_unchanged
 }
 
-- 
1.8.2.rc3.21.g744ac65

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks

2013-03-24 Thread John Keeping
On Sat, Mar 23, 2013 at 10:19:36PM -0700, Junio C Hamano wrote:
  In the longer term, difftool probably needs to learn to warn the user
  instead of overwrite any changes that have been made to the working tree
  file.
 
 Questionable.
 
 Admittedly I do not use difftool myself, and I have long assumed
 that difftool users are using the tools to _view_ the changes, but
 apparently some of the tools let the user muck with what is shown,
 and also apparently people seem to like the fact that they can make
 changes.  So I've led to believe the update in difftool, take the
 change back to working tree, either by making symbolic links or
 copying them back behaviour was a _feature_.

Yes it is.  I think my explanation wasn't clear enough here.

What currently happens is that after the user's tool has finished
running the working tree file and temporary file are compared and if
they are different then the temporary file is copied over the working
tree file.

This is good if the user has edited the temporary file, but what if they
edit they working tree file while using the tool to examine the
differences?  I think we need to at the very least look at the mtime of
the files and refuse to copy over the temporary file if that of the
working tree file is newer.

Obviously none of this matters if we can use symlinks, but in the
non-symlink case I think a user might find it surprising if the
(unmodified) file used by their diff tool were suddenly copied over the
working tree wiping out the changes they have just made.

 It is possible that this is not universally considerd as a feature,
 but if that is the case, I think the right way to do this is to tell
 the tools _not_ to let the user to modify contents they show in the
 first place, not letting the user modify and then warning after the
 fact.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] git-merge-one-file: revise merge error reporting

2013-03-24 Thread Kevin Bracey
Commit 718135e improved the merge error reporting for the resolve
strategy's merge conflict and permission conflict cases, but led to a
malformed ERROR:  in myfile.c message in the case of a file added
differently.

This commit reverts that change, and uses an alternative approach without
this flaw.

Signed-off-by: Kevin Bracey ke...@bracey.fi
---
 git-merge-one-file.sh | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 39b7799..e231d20 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -107,10 +107,12 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
 
+   ret=0
src2=$(git-unpack-file $3)
case $1 in
'')
-   echo Added $4 in both, but differently.
+   echo ERROR: Added $4 in both, but differently. 2
+   ret=1
orig=$(git-unpack-file $2)
create_virtual_base $orig $src2
;;
@@ -124,11 +126,10 @@ case ${1:-.}${2:-.}${3:-.} in
# would confuse merge greatly.
src1=$(git-unpack-file $2)
git merge-file $src1 $orig $src2
-   ret=$?
-   msg=
-   if test $ret != 0
+   if test $? != 0
then
-   msg='content conflict'
+   echo ERROR: Content conflict in $4 2
+   ret=1
fi
 
# Create the working tree file, using our tree version from the
@@ -138,21 +139,12 @@ case ${1:-.}${2:-.}${3:-.} in
 
if test $6 != $7
then
-   if test -n $msg
-   then
-   msg=$msg, 
-   fi
-   msg=${msg}permissions conflict: $5-$6,$7
-   ret=1
-   fi
-   if test -z $1
-   then
+   echo ERROR: Permissions conflict: $5-$6,$7 2
ret=1
fi
 
if test $ret != 0
then
-   echo ERROR: $msg in $4 2
exit 1
fi
exec git update-index -- $4
-- 
1.8.2.rc3.21.g744ac65

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] checkout: avoid unnecessary match_pathspec calls

2013-03-24 Thread Nguyễn Thái Ngọc Duy
In checkout_paths() we do this

 - for all updated items, call match_pathspec
 - for all items, call match_pathspec (inside unmerge_cache)
 - for all items, call match_pathspec (for showing path .. is unmerged)
 - for updated items, call match_pathspec and update paths

That's a lot of duplicate match_pathspec(s) and the function is not
exactly cheap to be called so many times, especially on large indexes.
This patch makes it call match_pathspec once per updated index entry,
save the result in ce_flags and reuse the results in the following
loops.

The changes in 0a1283b (checkout $tree $path: do not clobber local
changes in $path not in $tree - 2011-09-30) limit the affected paths
to ones we read from $tree. We do not do anything to other modified
entries in this case, so the for all items above could be modified
to for all updated items. But..

The command's behavior now is modified slightly: unmerged entries that
match $path, but not updated by $tree, are now NOT touched.  Although
this should be considered a bug fix, not a regression.

And while at there, free ps_matched after use.

The following command is tested on webkit, 215k entries. The pattern
is chosen mainly to make match_pathspec sweat:

git checkout -- *[a-zA-Z]*[a-zA-Z]*[a-zA-Z]*

before  after
real0m3.493s0m2.737s
user0m2.239s0m1.586s
sys 0m1.252s0m1.151s

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
  and worry about larger ones later, so if there were another choice,
  i.e.
 
   - eject nd/magic-pathspecs for now, cook this (and other small
 independent and clear improvements you may come up with, some of
 which might come out of nd/magic-pathspecs itself) and let it
 graduate first, and later revisit rerolld nd/magic-pathspecs
 
  that would be the ideal among the given choices ;-).

 Whichever is easier for you.

  The above is a faithful rewrite, but I have to wonder why you need
  two separate loops.
 
  Do you understand what the original loop is doing with ps_matched,
  and why the code excludes certain paths while doing so?

 Nope, I did not dig that deep. I expected you to do it ;-) j/k

  After commenting on the above, it makes me wonder if we even need to
  bother marking entries that were in the index that did not come from
  the tree-ish we are checking paths out of, though.  What breaks if
  you did not do the rewrite above and dropped the second loop in your
  patch?

 The test suite says none. There is a behavior change regarding
 unmerged entries as mentioned in the commit message. But I think it's
 a good change.

 builtin/checkout.c | 34 +++---
 cache.h|  1 +
 resolve-undo.c | 19 ++-
 resolve-undo.h |  1 +
 4 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..359b983 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -271,24 +271,46 @@ static int checkout_paths(const struct checkout_opts 
*opts,
;
ps_matched = xcalloc(1, pos);
 
+   /*
+* Make sure all pathspecs participated in locating the paths
+* to be checked out.
+*/
for (pos = 0; pos  active_nr; pos++) {
struct cache_entry *ce = active_cache[pos];
+   ce-ce_flags = ~CE_MATCHED;
if (opts-source_tree  !(ce-ce_flags  CE_UPDATE))
+   /*
+* git checkout tree-ish -- path, but this entry
+* is in the original index; it will not be checked
+* out to the working tree and it does not matter
+* if pathspec matched this entry.  We will not do
+* anything to this entry at all.
+*/
continue;
-   match_pathspec(opts-pathspec, ce-name, ce_namelen(ce), 0, 
ps_matched);
+   /*
+* Either this entry came from the tree-ish we are
+* checking the paths out of, or we are checking out
+* of the index.
+*/
+   if (match_pathspec(opts-pathspec, ce-name, ce_namelen(ce),
+  0, ps_matched))
+   ce-ce_flags |= CE_MATCHED;
}
 
-   if (report_path_error(ps_matched, opts-pathspec, opts-prefix))
+   if (report_path_error(ps_matched, opts-pathspec, opts-prefix)) {
+   free(ps_matched);
return 1;
+   }
+   free(ps_matched);
 
/* checkout -m path to recreate conflicted state */
if (opts-merge)
-   unmerge_cache(opts-pathspec);
+   unmerge_marked_index(the_index);
 
/* Any unmerged paths? */
for (pos = 0; pos  active_nr; pos++) {
struct cache_entry *ce = active_cache[pos];
-   if (match_pathspec(opts-pathspec, ce-name, ce_namelen(ce), 

Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks

2013-03-24 Thread Matt McClure
On Sun, Mar 24, 2013 at 1:19 AM, Junio C Hamano gits...@pobox.com wrote:
 Admittedly I do not use difftool myself, and I have long assumed
 that difftool users are using the tools to _view_ the changes, but
 apparently some of the tools let the user muck with what is shown,
 and also apparently people seem to like the fact that they can make
 changes.  So I've led to believe the update in difftool, take the
 change back to working tree, either by making symbolic links or
 copying them back behaviour was a _feature_.

Definitely. Here's how I use it:
http://matthewlmcclure.com/s/2013/03/14/use-emacs-dircmp-mode-as-a-git-difftool.html

-- 
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks

2013-03-24 Thread Matt McClure
On Sun, Mar 24, 2013 at 8:36 AM, John Keeping j...@keeping.me.uk wrote:
 In the
 non-symlink case I think a user might find it surprising if the
 (unmodified) file used by their diff tool were suddenly copied over the
 working tree wiping out the changes they have just made.

That's exactly what I was describing here:
http://thread.gmane.org/gmane.comp.version-control.git/217979/focus=218006

-- 
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/2] mergetools/p4merge: create a base if none available

2013-03-24 Thread Kevin Bracey
Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:

   p4merge $LOCAL $LOCAL $REMOTE $MERGED

Commit 0a0ec7bd changed this to:

   p4merge empty file $LOCAL $REMOTE $MERGED

to avoid the problem of being unable to save in some circumstances with
similar inputs.

Unfortunately this approach produces much worse results on differing
inputs. P4Merge really regards the blank file as the base, and once you
have just a couple of differences between the two branches you end up
with one a massive full-file conflict. The 3-way diff is not readable,
and you have to invoke difftool MERGE_HEAD HEAD manually to get a
useful view.

The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is  or
equal to the left input.  You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched the base,
it would autoresolve to the other branch).

But there appears to be no way of getting this 2-way behaviour and being
able to reliably save. Having base==left appears to be triggering other
assumptions. There are tricks the user can use to force the save icon
on, but it's not intuitive.

So we now follow a suggestion given in the original patch's discussion:
generate a virtual base, consisting of the lines common to the two
branches. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.

Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files.  As with the other merges using this
technique, we assume the user will inspect the result before saving.

Signed-off-by: Kevin Bracey ke...@bracey.fi
Reviewed-by: David Aguilar dav...@gmail.com
---
Minor change from v3: that version moved initialisation of src1 higher up,
detaching it from its associated comment. This move was only required by
earlier versions, so v4 leaves src1 in its original position.

Added Reviewed-by footer.

 Documentation/git-sh-setup.txt |  6 ++
 git-merge-one-file.sh  | 18 +-
 git-sh-setup.sh| 12 
 mergetools/p4merge |  6 +-
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
 
+create_virtual_base::
+   modifies the first file so only lines in common with the
+   second file remain. If there is insufficient common material,
+   then the first file is left empty. The result is suitable
+   as a virtual base input for a 3-way merge.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 3373c04..255c07a 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case ${1:-.}${2:-.}${3:-.} in
;;
esac
 
-   src2=`git-unpack-file $3`
+   src2=$(git-unpack-file $3)
case $1 in
'')
echo Added $4 in both, but differently.
-   # This extracts OUR file in $orig, and uses git apply to
-   # remove lines that are unique to ours.
-   orig=`git-unpack-file $2`
-   sz0=`wc -c $orig`
-   @@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
-   sz1=`wc -c $orig`
-
-   # If we do not have enough common material, it is not
-   # worth trying two-file merge using common subsections.
-   expr $sz0 \ $sz1 \* 2 /dev/null || : $orig
+   orig=$(git-unpack-file $2)
+   create_virtual_base $orig $src2
;;
*)
echo Auto-merging $4
-   orig=`git-unpack-file $1`
+   orig=$(git-unpack-file $1)
;;
esac
 
# Be careful for funny filename such as -L in $4, which
# would confuse merge greatly.
-   src1=`git-unpack-file $2`
+   src1=$(git-unpack-file $2)
git merge-file $src1 $orig $src2
ret=$?
msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 9cfbe7f..2f78359 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+   sz0=$(wc -c $1)
+   @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+   sz1=$(wc -c $1)
+
+   # If we do not have enough common material, it 

Re: [PATCH 4/4] transport: drop int cmp = cmp hack

2013-03-24 Thread Torsten Bögershausen
On 24.03.13 10:32, Jeff King wrote:
 On Sat, Mar 23, 2013 at 09:00:05PM -0700, Junio C Hamano wrote:
 
 On Thu, Mar 21, 2013 at 4:13 AM, Jeff King p...@peff.net wrote:

 According to 47ec794, this initialization is meant to
 squelch an erroneous uninitialized variable warning from gcc
 4.0.1.  That version is quite old at this point, and gcc 4.1
 and up handle it fine, with one exception. There seems to be
 a regression in gcc 4.6.3, which produces the warning;
 however, gcc versions 4.4.7 and 4.7.2 do not.


 transport.c: In function 'get_refs_via_rsync':
 transport.c:127:29: error: 'cmp' may be used uninitialized in this
 function [-Werror=uninitialized]
 transport.c:109:7: note: 'cmp' was declared here

 gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
 
 Right, that's the same version I noted above. Is 4.6.3 the default
 compiler under a particular release of Ubuntu, or did you use their
 gcc-4.6 package?
 
 -Peff
Side question:
How much does it hurt to write like this:

diff --git a/transport.c b/transport.c
index 6b2ae94..8020b62 100644
--- a/transport.c
+++ b/transport.c
@@ -106,7 +106,7 @@ static void insert_packed_refs(const char *packed_refs, 
struct ref **list)
return;
 
for (;;) {
-   int cmp, len;
+   int cmp=0, len;
 
==

Using Ubuntu 10.4, using gcc (Ubuntu 4.4.3-4ubuntu5.1) 4.4.3
the compiler will add a line like this:

  2e83: 31 ff   xor%edi,%edi

(Which should not be to slow to execute)

Looking at a later gcc, from upcoming Debian, with gcc (Debian 4.7.2-5) 4.7.2
the assembly code is exactly the same ;-)







--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks

2013-03-24 Thread John Keeping
On Sun, Mar 24, 2013 at 09:31:45AM -0400, Matt McClure wrote:
 On Sun, Mar 24, 2013 at 8:36 AM, John Keeping j...@keeping.me.uk wrote:
  In the
  non-symlink case I think a user might find it surprising if the
  (unmodified) file used by their diff tool were suddenly copied over the
  working tree wiping out the changes they have just made.
 
 That's exactly what I was describing here:
 http://thread.gmane.org/gmane.comp.version-control.git/217979/focus=218006

Ahh, I guess I didn't fully register the impact of that at the time and
had to rediscover the problem for myself ;-)

How about doing this (on top of jk/difftool-dir-diff-edit-fix)?

-- 8 --
Subject: [PATCH] difftool: don't overwrite modified files

After running the user's diff tool, git-difftool will copy any files
that differ between the working tree and the temporary tree.  This is
useful when the user edits the file in their diff tool but is wrong if
they edit the working tree file while examining the diff.

Instead of copying unconditionally when the files differ, store the
initial hash of the working tree file and only copy the temporary file
back if it was modified and the working tree file was not.  If both
files have been modified, print a warning and exit with an error.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 git-difftool.perl   | 35 +--
 t/t7800-difftool.sh | 26 ++
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index c433e86..be82b5a 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -15,7 +15,6 @@ use strict;
 use warnings;
 use File::Basename qw(dirname);
 use File::Copy;
-use File::Compare;
 use File::Find;
 use File::stat;
 use File::Path qw(mkpath rmtree);
@@ -123,7 +122,7 @@ sub setup_dir_diff
my $rindex = '';
my %submodule;
my %symlink;
-   my @working_tree = ();
+   my %working_tree;
my @rawdiff = split('\0', $diffrtn);
 
my $i = 0;
@@ -175,7 +174,9 @@ EOF
 
if ($rmode ne $null_mode) {
if (use_wt_file($repo, $workdir, $dst_path, $rsha1, 
$symlinks)) {
-   push(@working_tree, $dst_path);
+   $working_tree{$dst_path} =
+   $repo-command_oneline('hash-object',
+   $workdir/$dst_path);
} else {
$rindex .= $rmode $rsha1\t$dst_path\0;
}
@@ -227,7 +228,7 @@ EOF
# not part of the index. Remove any trailing slash from $workdir
# before starting to avoid double slashes in symlink targets.
$workdir =~ s|/$||;
-   for my $file (@working_tree) {
+   for my $file (keys %working_tree) {
my $dir = dirname($file);
unless (-d $rdir/$dir) {
mkpath($rdir/$dir) or
@@ -278,7 +279,7 @@ EOF
exit_cleanup($tmpdir, 1) if not $ok;
}
 
-   return ($ldir, $rdir, $tmpdir, @working_tree);
+   return ($ldir, $rdir, $tmpdir, %working_tree);
 }
 
 sub write_to_file
@@ -376,7 +377,7 @@ sub dir_diff
my $error = 0;
my $repo = Git-repository();
my $workdir = find_worktree($repo);
-   my ($a, $b, $tmpdir, @worktree) =
+   my ($a, $b, $tmpdir, %worktree) =
setup_dir_diff($repo, $workdir, $symlinks);
 
if (defined($extcmd)) {
@@ -390,19 +391,25 @@ sub dir_diff
# should be copied back to the working tree.
# Do not copy back files when symlinks are used and the
# external tool did not replace the original link with a file.
-   for my $file (@worktree) {
+   for my $file (keys %worktree) {
next if $symlinks  -l $b/$file;
next if ! -f $b/$file;
 
-   my $diff = compare($b/$file, $workdir/$file);
-   if ($diff == 0) {
-   next;
-   } elsif ($diff == -1) {
-   my $errmsg = warning: Could not compare ;
-   $errmsg += '$b/$file' with '$workdir/$file'\n;
+   my $wt_hash = $repo-command_oneline('hash-object',
+   $workdir/$file);
+   my $tmp_hash = $repo-command_oneline('hash-object',
+   $b/$file);
+   my $wt_modified = $wt_hash ne $worktree{$file};
+   my $tmp_modified = $tmp_hash ne $worktree{$file};
+
+   if ($wt_modified and $tmp_modified) {
+   my $errmsg = warning: Both files modified: ;
+   $errmsg .= '$workdir/$file' and '$b/$file'.\n;
+   $errmsg .= warning: Working tree file has been 
left.\n;
+   $errmsg .= warning:\n;
warn $errmsg;
$error = 1;
-   } elsif ($diff == 1) {
+   

Why does 'submodule add' stage the relevant portions?

2013-03-24 Thread Ramkumar Ramachandra
Hi,

I find this behavior very inconsistent and annoying.  Why would I want
to commit the submodule change immediately?  Maybe I want to batch it
up with other changes and stage it at a later time.  Why should I have
to unstage them manually now?  I get the other side of the argument:
what if the user commits the .gitmodule change at a different time
from the file change?  In other words, the user should have a way of
saying 'submodule stage' and 'submodule unstage'.

Now, for the implementation.  Do we have existing infrastructure to
stage a hunk non-interactively?  (The ability to select a hunk to
stage/ unstage programmatically).  If not, it might be quite a
non-trivial thing to write.

Git 2.0 is coming soon, so I'm excited about breaking a lot of
backward compatibility ;)

Thanks.

Ram
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

2013-03-24 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

 You should always trust but verify what other commands do at key
 points of the operation; and I think this require-clean-work-tree
 is a key precondition for this mode of operation to work correctly.

 Especially because you do not even bother to check the result of
 git stash before continuing ;-).

If you think it's enough to replicate the codepath that precedes the
actual saving in 'git stash' (which is essentially
require-clean-work-tree), I'm in agreement with you.  I thought you
were implying a wider safety net, that wouldn't even assume the basic
sanity of stash.

 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

 Delaying to run git stash as much as possible is fine, but with
 the above, can the user tell if something was stashed and she has
 to stash pop once she is done helping the command to resolve the
 conflicts, or she shouldn't run stash pop after she is done
 (because if nothing is stashed at this point, that pop will pop an
 unrelated stash)?

 I could easily tell, from the git pull output: if conflict, then
 stash hasn't been applied.

 The question was about git stash save.  Depending on the cleanness
 of the tree when git pull was started, save may or may not have
 been done.  After resolving a conflicted git pull and committing
 the result, the user does not have much clue if she needs to pop
 anything, does she?  Instead of the usual resolve the conflicts and
 commit the result, she may need to see resolve the conflicts,
 commit the result, *AND* UNSTASH.

Yes, good point.  Will it suffice to print a message?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] checkout: add --sparse for restoring files in sparse checkout mode

2013-03-24 Thread Jonathan Nieder
Hi,

Nguyễn Thái Ngọc Duy wrote:

 --- a/Documentation/git-checkout.txt
 +++ b/Documentation/git-checkout.txt
 @@ -180,6 +180,13 @@ branch by running git rm -rf . from the top level of 
 the working tree.
  Afterwards you will be ready to prepare your new files, repopulating the
  working tree, by copying them from elsewhere, extracting a tarball, etc.

 +
 +--sparse::
 + In sparse checkout mode, `git checkout -- paths` would
 + update all entries matched by paths regardless sparse
 + patterns. This option only updates entries matched by paths
 + and sparse patterns.

Hm, should this be the default?

In principle, I would expect

git checkout -- .

to make the worktree match the index, respecting the sparse checkout.
And something like

git checkout --widen -- .

to change the sparse checkout pattern.  But of course it is easily
possible that I am missing some details of how sparse checkout is
used in practice.

What do you think?
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


propagating repo corruption across clone

2013-03-24 Thread Jeff King
I saw this post-mortem on recent disk corruption seen on git.kde.org:

  http://jefferai.org/2013/03/24/too-perfect-a-mirror/

The interesting bit to me is that object corruption propagated across a
clone (and oddly, that --mirror made complaints about corruption go
away). I did a little testing and found some curious results (this ended
up long; skip to the bottom for my conclusions).

Here's a fairly straight-forward corruption recipe:

-- 8 --
obj_to_file() {
  echo .git/objects/$(echo $1 | sed 's,..,/,')
}

# corrupt a single byte inside the object
corrupt_object() {
  fn=$(obj_to_file $1) 
  chmod +w $fn 
  printf '\0' | dd of=$fn bs=1 conv=notrunc seek=10
}

git init repo 
cd repo 
echo content file 
git add file 
git commit -m one 
corrupt_object $(git rev-parse HEAD:file)
-- 8 --

report git clone . fast-local
report git clone --no-local . no-local
report git -c transfer.unpackLimit=1 clone --no-local . index-pack
report git -c fetch.fsckObjects=1 clone --no-local . fsck

and here is how clone reacts in a few situations:

  $ git clone --bare . local-bare  echo WORKED
  Cloning into bare repository 'local-bare'...
  done.
  WORKED

We don't notice the problem during the transport phase, which is to be
expected; we're using the fast just hardlink it code path. So that's
OK.

  $ git clone . local-tree  echo WORKED
  Cloning into 'local-tree'...
  done.
  error: inflate: data stream error (invalid distance too far back)
  error: unable to unpack d95f3ad14dee633a758d2e331151e950dd13e4ed header
  WORKED

We _do_ see a problem during the checkout phase, but we don't propagate
a checkout failure to the exit code from clone.  That is bad in general,
and should probably be fixed. Though it would never find corruption of
older objects in the history, anyway, so checkout should not be relied
on for robustness.

  $ git clone --no-local . non-local  echo WORKED
  Cloning into 'non-local'...
  remote: Counting objects: 3, done.
  remote: error: inflate: data stream error (invalid distance too far back)
  remote: error: unable to unpack d95f3ad14dee633a758d2e331151e950dd13e4ed 
header
  remote: error: inflate: data stream error (invalid distance too far back)
  remote: fatal: loose object d95f3ad14dee633a758d2e331151e950dd13e4ed (stored 
in ./objects/d9/5f3ad14dee633a758d2e331151e950dd13e4ed) is corrupt
  error: git upload-pack: git-pack-objects died with error.
  fatal: git upload-pack: aborting due to possible repository corruption on the 
remote side.
  remote: aborting due to possible repository corruption on the remote side.
  fatal: early EOF
  fatal: index-pack failed

Here we detect the error. It's noticed by pack-objects on the remote
side as it tries to put the bogus object into a pack. But what if we
already have a pack that's been corrupted, and pack-objects is just
pushing out entries without doing any recompression?

Let's change our corrupt_object to:

  corrupt_object() {
git repack -ad 
pack=`echo .git/objects/pack/*.pack` 
chmod +w $pack 
printf '\0' | dd of=$pack bs=1 conv=notrunc seek=175
  }

and try again:

  $ git clone --no-local . non-local  echo WORKED
  Cloning into 'non-local'...
  remote: Counting objects: 3, done.
  remote: Total 3 (delta 0), reused 3 (delta 0)
  error: inflate: data stream error (invalid distance too far back)
  fatal: pack has bad object at offset 169: inflate returned -3
  fatal: index-pack failed

Great, we still notice the problem in unpack-objects on the receiving
end. But what if there's a more subtle corruption, where filesystem
corruption points the directory entry for one object at the inode of
another. Like:

  corrupt_object() {
corrupt=$(echo corrupted | git hash-object -w --stdin) 
mv -f $(obj_to_file $corrupt) $(obj_to_file $1)
  }

This is going to be more subtle, because the object in the packfile is
self-consistent but the object graph as a whole is broken.

  $ git clone --no-local . non-local  echo WORKED
  Cloning into 'non-local'...
  remote: Counting objects: 3, done.
  remote: Total 3 (delta 0), reused 0 (delta 0)
  Receiving objects: 100% (3/3), done.
  error: unable to find d95f3ad14dee633a758d2e331151e950dd13e4ed
  WORKED

Like the --local cases earlier, we notice the missing object during the
checkout phase, but do not correctly propagate the error.

We do not notice the sha1 mis-match on the sending side (which we could,
if we checked the sha1 as we were sending). We do not notice the broken
object graph during the receive process either. I would have expected
check_everything_connected to handle this, but we don't actually call it
during clone! If you do this:

  $ git init non-local  cd non-local  git fetch ..
  remote: Counting objects: 3, done.
  remote: Total 3 (delta 0), reused 3 (delta 0)
  Unpacking objects: 100% (3/3), done.
  fatal: missing blob object 'd95f3ad14dee633a758d2e331151e950dd13e4ed'
  error: .. did not send all necessary objects

we do notice.

And one final check:

  $ git -c 

Re: propagating repo corruption across clone

2013-03-24 Thread Ævar Arnfjörð Bjarmason
On Sun, Mar 24, 2013 at 7:31 PM, Jeff King p...@peff.net wrote:

 I don't have details on the KDE corruption, or why it wasn't detected
 (if it was one of the cases I mentioned above, or a more subtle issue).

One thing worth mentioning is this part of the article:

Originally, mirrored clones were in fact not used, but non-mirrored
clones on the anongits come with their own set of issues, and are more
prone to getting stopped up by legitimate, authenticated force pushes,
ref deletions, and so on – and if we set the refspec such that those
are allowed through silently, we don’t gain much. 

So the only reason they were even using --mirror was because they were
running into those problems with fetching.

So aside from the problems with --mirror I think we should have
something that updates your local refs to be exactly like they are on
the other end, i.e. deletes some, non-fast-forwards others etc.
(obviously behind several --force options and so on). But such an
option *wouldn't* accept corrupted objects.

That would give KDE and other parties a safe way to do exact repo
mirroring like this, wouldn't protect them from someone maliciously
deleting all the refs in all the repos, but would prevent FS
corruption from propagating.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: propagating repo corruption across clone

2013-03-24 Thread Jeff King
On Sun, Mar 24, 2013 at 08:01:33PM +0100, Ævar Arnfjörð Bjarmason wrote:

 On Sun, Mar 24, 2013 at 7:31 PM, Jeff King p...@peff.net wrote:
 
  I don't have details on the KDE corruption, or why it wasn't detected
  (if it was one of the cases I mentioned above, or a more subtle issue).
 
 One thing worth mentioning is this part of the article:
 
 Originally, mirrored clones were in fact not used, but non-mirrored
 clones on the anongits come with their own set of issues, and are more
 prone to getting stopped up by legitimate, authenticated force pushes,
 ref deletions, and so on – and if we set the refspec such that those
 are allowed through silently, we don’t gain much. 
 
 So the only reason they were even using --mirror was because they were
 running into those problems with fetching.

I think the --mirror thing is a red herring. It should not be changing
the transport used, and that is the part of git that is expected to
catch such corruption.

But I haven't seen exactly what the corruption is, nor exactly what
commands they used to clone. I've invited the blog author to give more
details in this thread.

 So aside from the problems with --mirror I think we should have
 something that updates your local refs to be exactly like they are on
 the other end, i.e. deletes some, non-fast-forwards others etc.
 (obviously behind several --force options and so on). But such an
 option *wouldn't* accept corrupted objects.

That _should_ be how git fetch --prune +refs/*:refs/* behaves (and
that refspec is set up when you use --mirror; we should probably have
it turn on --prune, too, but I do not think you can do so via a config
option currently).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: propagating repo corruption across clone

2013-03-24 Thread Ilari Liusvaara
On Sun, Mar 24, 2013 at 02:31:33PM -0400, Jeff King wrote:
 
 Fscking the incoming objects does work, but of course it comes at a cost
 in the normal case (for linux-2.6, I measured an increase in CPU time
 with index-pack --strict from ~2.5 minutes to ~4 minutes). And I think
 it is probably overkill for finding corruption; index-pack already
 recognizes bit corruption inside an object, and
 check_everything_connected can detect object graph problems much more
 cheaply.

AFAIK, standard checks index-pack has to do + checking that the object
graph has no broken links (and every ref points to something valid) will
catch everything except:

- SHA-1 collisions between corrupt objects and clean ones.
- Corrupted refs (that still point to something valid).
- Born-corrupted objects.

 One thing I didn't check is bit corruption inside a packed object that
 still correctly zlib inflates. check_everything_connected will end up
 reading all of the commits and trees (to walk them), but not the blobs.

Checking that everything is connected will (modulo SHA-1 collisions) save
you there, at least if packv3 is used as transport stream.

-Ilari
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] contrib/subtree: stop explicitly using a bash shell

2013-03-24 Thread Paul Campbell
Don't explicitly use the Bash shell but allow the system to provide a
hopefully POSIX compatible shell at /bin/sh.

Signed-off-by: Paul Campbell pcampb...@kemitix.net
---

Only the system's I was able to test this on (Debian squeeze) /bin/sh is
the dash shell.

 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..5701376 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 #
 # git-subtree.sh: split/join git repositories in subdirectories of this one
 #
-- 
1.8.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands

2013-03-24 Thread Paul Campbell
Use of -a and -o in the [ command can have confusing semantics.

Use a separate test invocation for each single test, combining them with
 and ||, and use ordinary parentheses for grouping.

Signed-off-by: Paul Campbell pcampb...@kemitix.net
---
 contrib/subtree/git-subtree.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5701376..884cbfb 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -119,7 +119,7 @@ esac
 
 dir=$(dirname $prefix/.)
 
-if [ $command != pull -a $command != add -a $command != push ]; 
then
+if ( test $command != pull )  ( test $command != add )  ( test 
$command != push ); then
revs=$(git rev-parse $default --revs-only $@) || exit $?
dirs=$(git rev-parse --no-revs --no-flags $@) || exit $?
if [ -n $dirs ]; then
@@ -181,9 +181,9 @@ cache_set()
 {
oldrev=$1
newrev=$2
-   if [ $oldrev != latest_old \
--a $oldrev != latest_new \
--a -e $cachedir/$oldrev ]; then
+   if ( test $oldrev != latest_old ) \
+ ( test $oldrev != latest_new ) \
+ ( test -e $cachedir/$oldrev ); then
die cache for $oldrev already exists!
fi
echo $newrev $cachedir/$oldrev
@@ -273,12 +273,12 @@ find_existing_splits()
git-subtree-split:) sub=$b ;;
END)
debug   Main is: '$main'
-   if [ -z $main -a -n $sub ]; then
+   if ( test -z $main )  ( test -n $sub ); 
then
# squash commits refer to a subtree
debug   Squash: $sq from $sub
cache_set $sq $sub
fi
-   if [ -n $main -a -n $sub ]; then
+   if ( test -n $main )  (test -n $sub ); 
then
debug   Prior: $main - $sub
cache_set $main $sub
cache_set $sub $sub
@@ -541,7 +541,7 @@ cmd_add_commit()
tree=$(git write-tree) || exit $?

headrev=$(git rev-parse HEAD) || exit $?
-   if [ -n $headrev -a $headrev != $rev ]; then
+   if ( test -n $headrev )  ( test $headrev != $rev ); then
headp=-p $headrev
else
headp=
-- 
1.8.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git-svn: branch trunk/project/code to branches/project-1.0/code

2013-03-24 Thread rupert THURNER
our subversion repository contains various things, which are in
dedicated subdirectories:
trunk/project/code
trunk/project/doc
trunk/project/...

the same is then valid for the branches:
branches/project-1.0/code
branches/project-1.0/doc
branches/project-1.0/...

as there are many folders with big items i tried to only track the
code part, but with branches. i though one can just have something
like:

[svn-remote svn]
url = url-of-svn-repository
fetch = trunk/project/code:refs/remotes/trunk
branches = branches/project-*/code:refs/remotes/project-*

but this is not correct. how could one support such branches?

 rupert
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] checkout: add --sparse for restoring files in sparse checkout mode

2013-03-24 Thread Kirill Müller

Hi

On 03/24/2013 07:17 PM, Jonathan Nieder wrote:

Hi,

Nguyễn Thái Ngọc Duy wrote:


--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -180,6 +180,13 @@ branch by running git rm -rf . from the top level of the 
working tree.
  Afterwards you will be ready to prepare your new files, repopulating the
  working tree, by copying them from elsewhere, extracting a tarball, etc.

+
+--sparse::
+   In sparse checkout mode, `git checkout -- paths` would
+   update all entries matched by paths regardless sparse
+   patterns. This option only updates entries matched by paths
+   and sparse patterns.

Hm, should this be the default?

In principle, I would expect

git checkout -- .

to make the worktree match the index, respecting the sparse checkout.
And something like

git checkout --widen -- .

to change the sparse checkout pattern.  But of course it is easily
possible that I am missing some details of how sparse checkout is
used in practice.

What do you think?
Thank you for your opinion. I'd second that. When I do a sparse 
checkout, I want to see only the directories I have included and not 
excluded, unless I explicitly change that.



-Kirill
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] user-manual: Fix the interactive rebase example commit range

2013-03-24 Thread Eric Sunshine
On Sun, Mar 24, 2013 at 8:23 AM, W. Trevor King wk...@tremily.us wrote:
 6c26bf4 (user-manual: Reorganize the reroll sections, adding 'git
 rebase -i', 2013-02-19) used deadbee as the oldest commit in the pick
 list, but as the final commit in the rebased range, due to sloppy
 duplication and extension from git-rebase.txt.  This fixes that by
 adding a new HEAD commit.

 I also reworded the example commit summaries (onelines), because I
 think my initial mistake may have been to to misinterpreting this

s/to to/due to/

 commit as HEAD without thinking things through ;).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/23] contrib/subtree: Teach add to store repository branch in .gittrees

2013-03-24 Thread Paul Campbell
On Mon, Mar 11, 2013 at 3:24 AM, Junio C Hamano gits...@pobox.com wrote:
 Paul Campbell pcampb...@kemitix.net writes:

 From: Matt Hoffman matt.hoff...@quantumretail.com

 The repository and branch of a subtree added with the add command is
 stored in the .gittrees file.

 Signed-off-by: Paul Campbell pcampb...@kemitix.net
 ---
  contrib/subtree/git-subtree.sh | 8 
  1 file changed, 8 insertions(+)

 diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
 index caf4988..7b70251 100755
 --- a/contrib/subtree/git-subtree.sh
 +++ b/contrib/subtree/git-subtree.sh
 @@ -528,6 +528,14 @@ cmd_add_repository()
   revs=FETCH_HEAD
   set -- $revs
   cmd_add_commit $@
 +
 +  # now add it to our list of repos
 +  git config -f .gittrees --unset subtree.$dir.url
 +  git config -f .gittrees --add subtree.$dir.url $repository
 +  git config -f .gittrees --unset subtree.$dir.path
 +  git config -f .gittrees --add subtree.$dir.path $dir
 +  git config -f .gittrees --unset subtree.$dir.branch
 +  git config -f .gittrees --add subtree.$dir.branch $refspec

 Existing code in the function this touches seem to be written
 carefully to allow $IFS whitespace in $dir, but this change butchers
 it, it seems.

 Also, where does $refspec come from?  When this is called from
 cmd_add_repository(), there is an assignment to the variable, but it
 is not all clear.  As git-subtree declares it won't work with
 anything but bash, I think things like this should take advantage of
 being written for bash by using local and passing arguments
 explicitly instead of relying on global variables, which POSIX shell
 scripts cannot afford to do but bash scripts can.

  }

  cmd_add_commit()

$refspec gets assigned in the third line of the function. The function
has only one caller in cmd_add() inside an if clause that requires
there be two parameters.

Personally I'd rather move git-subtree to be more portable and less
dependant on bash. I've sent some patches earlier to remove that
dependancy. As far as I could see there wasn't really anything that
was bash-only.

Reroll of this patch coming shortly allowing for $IFS whitespace.

-- 
Paul [W] Campbell
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] user-manual: Fix the interactive rebase example commit range

2013-03-24 Thread W. Trevor King
On Sun, Mar 24, 2013 at 04:00:17PM -0400, Eric Sunshine wrote:
 On Sun, Mar 24, 2013 at 8:23 AM, W. Trevor King wk...@tremily.us wrote:
  I also reworded the example commit summaries (onelines), because I
  think my initial mistake may have been to to misinterpreting this
 
 s/to to/due to/

Oops, thanks :).


signature.asc
Description: OpenPGP digital signature


[PATCH] contrib/subtree: Teach add to store repository branch in .gittrees

2013-03-24 Thread Paul Campbell
From: Matt Hoffman matt.hoff...@quantumretail.com

The repository and branch of a subtree added with the add command is
stored in the .gittrees file.

Signed-off-by: Paul Campbell pcampb...@kemitix.net
---

Rerolled allowing for $IFS whitespace.

 contrib/subtree/git-subtree.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 8a23f58..b6e821e 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -527,6 +527,14 @@ cmd_add_repository()
revs=FETCH_HEAD
set -- $revs
cmd_add_commit $@
+
+   # now add it to our list of repos
+   git config -f .gittrees --unset subtree.$dir.url
+   git config -f .gittrees --add subtree.$dir.url $repository
+   git config -f .gittrees --unset subtree.$dir.path
+   git config -f .gittrees --add subtree.$dir.path $dir
+   git config -f .gittrees --unset subtree.$dir.branch
+   git config -f .gittrees --add subtree.$dir.branch $refspec
 }
 
 cmd_add_commit()
-- 
1.8.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] contrib/subtree: Add command from-submodule

2013-03-24 Thread Paul Campbell
Converts a git-submodule into a git-subtree.

Based-on-patch-by: Peter Jaros pja...@pivotallabs.com
Signed-off-by: Paul Campbell pcampb...@kemitix.net
---
 contrib/subtree/git-subtree.sh | 37 +++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index a81dfef..5e3c0b8 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -14,6 +14,7 @@ git subtree merge --prefix=prefix commit
 git subtree pull  --prefix=prefix repository refspec...
 git subtree push  --prefix=prefix repository refspec...
 git subtree split --prefix=prefix commit...
+git subtree from-submodule --prefix=prefix
 --
 h,helpshow the help
 q quiet
@@ -114,7 +115,7 @@ prefix=${prefix%/};
 command=$1
 shift
 case $command in
-   add|merge|pull) default= ;;
+   add|merge|pull|from-submodule) default= ;;
split|push) default=--default HEAD ;;
*) die Unknown command '$command' ;;
 esac
@@ -140,7 +141,14 @@ if ( test $command != pull )  ( test $command != 
add )  ( test $com
fi
 fi
 
+# map command to a function
+case $command in
+   from-submodule) cmd=cmd_from_submodule;;
+   *) cmd=cmd_$command;;
+esac
+
 debug command: {$command}
+debug function: {$function}
 debug quiet: {$quiet}
 debug revs: {$revs}
 debug dir: {$dir}
@@ -766,4 +774,29 @@ cmd_push()
fi
 }
 
-cmd_$command $@
+cmd_from_submodule()
+{
+   ensure_clean
+
+   # Remove references to submodule.
+   git config --remove-section submodule.$prefix
+   git config --file .gitmodules --remove-section submodule.$prefix
+   git add .gitmodules
+
+   # Move submodule aside.
+   tmp_repo=$(mktemp -d git-subtree.X)
+   rm -r $tmp_repo
+   mv $prefix $tmp_repo
+   git rm $prefix
+
+   # Commit changes.
+   git commit -m Remove '$prefix/' submodule
+
+   # subtree add from submodule repo.
+   cmd_add_repository $tmp_repo HEAD
+
+   # Remove submodule repo.
+   rm -rf $tmp_repo
+}
+
+$cmd $@
-- 
1.8.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug: git web--browse doesn't recognise browser on OS X

2013-03-24 Thread Christian Couder
On Fri, Mar 22, 2013 at 6:36 PM, John Szakmeister j...@szakmeister.net wrote:
 On Fri, Mar 22, 2013 at 12:19 PM, Timo Sand timo.j.s...@gmail.com wrote:
 Hi,

 well my use case is actually that I'm trying to use the gem
 'gem-browse' which uses 'git web--browse'
 I'm not using Apple Terminal, I'm using iTerm2 and there doesn't seem
 to be a SECURITYSESSIONID set, at least echo didn't find any. But
 neither did I find it on Apple Terminal either.

 I noticed this the other day, but I think SECURITYSESSIONID only gets
 set when Screen Sharing is enabled.  I had Screen Sharing enabled,
 launched iTerm2 and saw the variable.  I closed iTerm2, turned off
 Screen Sharing, and relaunched iTerm2 to find the variable missing.
 As a result, I'm not SECURITYSESSIONID is a good mechanism for
 determining whether the terminal is associated a GUI or not.
 Unfortunately, I don't know of a better way.

 What troubles me is that this issue has only arisen recently, earlier
 this worked fine for me

 The following patch fixes the issue by recognizing iTerm2 as a GUI terminal.

Your patch looks good to me, and I cannot really test it as I don't have a Mac.
Could you just had some of the explanations you gave above to the
commit message?

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 00/15] Use test_config

2013-03-24 Thread Yann Droneaud
Please find some patches to use test_config/test_unconfig

Instead of using construct such as:
   test_when_finished git config --unset key
   git config key value
uses
   test_config key value
The latter takes care of removing key at the end of the test.

Additionally, instead of
   git config key 
or
   git config --unset key
 uses
   test_unconfig key
The latter doesn't failed if key is not defined.

Patch t7600: use test_config to set/unset git config variables
is more important than the other and must be carefully reviewed
regarded to the --no-log --no-ff behavior.

Others patches are fairly simple.

Testsuite results are the same after the patches.
Tested against master, 7b592fadf1e23b10b913e0771b9f711770597266

Yann Droneaud (15):
  t4018: remove test_config implementation
  t7810: remove test_config implementation
  t7811: remove test_config implementation
  t3400: use test_config to set/unset git config variables
  t4304: use test_config to set/unset git config variables
  t4034: use test_config/test_unconfig to set/unset git config
variables
  t4202: use test_config/test_unconfig to set/unset git config
variables
  t5520: use test_config to set/unset git config variables
  t5541: use test_config to set/unset git config variables
  t7500: use test_config to set/unset git config variables
  t7502: use test_config to set/unset git config variables
  t7508: use test_config to set/unset git config variables
  t7600: use test_config to set/unset git config variables
  t9500: use test_config to set/unset git config variables
  t7502: remove clear_config

 t/t3400-rebase.sh  |  3 +-
 t/t3404-rebase-interactive.sh  |  3 +-
 t/t4018-diff-funcname.sh   |  5 ---
 t/t4034-diff-words.sh  |  7 ++--
 t/t4202-log.sh | 28 +---
 t/t5520-pull.sh| 12 +++
 t/t5541-http-push.sh   |  3 +-
 t/t7500-commit.sh  |  6 ++--
 t/t7502-commit.sh  | 40 +--
 t/t7508-status.sh  | 46 +-
 t/t7600-merge.sh   | 60 +++---
 t/t7810-grep.sh|  5 ---
 t/t7811-grep-open.sh   |  5 ---
 t/t9500-gitweb-standalone-no-errors.sh |  3 +-
 14 files changed, 72 insertions(+), 154 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/15] t4018: remove test_config implementation

2013-03-24 Thread Yann Droneaud
test_config is provided by the test library,
a private version is not needed.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t4018-diff-funcname.sh | 5 -
 1 file changed, 5 deletions(-)

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 082d3e8..38a092a 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -93,11 +93,6 @@ sed -e '
s/song;/song();/
 ' Beer.perl Beer-correct.perl
 
-test_config () {
-   git config $1 $2 
-   test_when_finished git config --unset $1
-}
-
 test_expect_funcname () {
lang=${2-java}
test_expect_code 1 git diff --no-index -U1 \
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/15] t7810: remove test_config implementation

2013-03-24 Thread Yann Droneaud
test_config is provided by the test library,
a private version is not needed.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7810-grep.sh | 5 -
 1 file changed, 5 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f698001..500eb50 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1084,11 +1084,6 @@ test_expect_success 'grep -E pattern with 
grep.patternType=fixed' '
test_cmp expected actual
 '
 
-test_config() {
-   git config $1 $2 
-   test_when_finished git config --unset $1
-}
-
 cat expected EOF
 hello.cRED:RESETint main(int argc, const char **argv)
 hello.cRED-RESET{
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/15] t3400: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t3400-rebase.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 1de0ebd..f6cc102 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -138,8 +138,7 @@ test_expect_success 'rebase a single mode change' '
 '
 
 test_expect_success 'rebase is not broken by diff.renames' '
-   git config diff.renames copies 
-   test_when_finished git config --unset diff.renames 
+   test_config diff.renames copies 
git checkout filemove 
GIT_TRACE=1 git rebase force-3way
 '
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 07/15] t4202: use test_config/test_unconfig to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Additionally, instead of
 git config key 
or
 git config --unset key
uses
 test_unconfig key
The latter doesn't failed if key is not defined.

Tests are modified to assume correct (default) configuration at entry,
and to reset the modified configuration variables at the end.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t4202-log.sh | 28 
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index fa686b8..9243a97 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -419,8 +419,6 @@ test_expect_success 'log --graph with merge' '
 '
 
 test_expect_success 'log.decorate configuration' '
-   test_might_fail git config --unset-all log.decorate 
-
git log --oneline expect.none 
git log --oneline --decorate expect.short 
git log --oneline --decorate=full expect.full 
@@ -429,8 +427,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline actual 
test_cmp expect.short actual 
 
-   git config --unset-all log.decorate 
-   git config log.decorate true 
+   test_config log.decorate true 
git log --oneline actual 
test_cmp expect.short actual 
git log --oneline --decorate=full actual 
@@ -438,8 +435,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --decorate=no actual 
test_cmp expect.none actual 
 
-   git config --unset-all log.decorate 
-   git config log.decorate no 
+   test_config log.decorate no 
git log --oneline actual 
test_cmp expect.none actual 
git log --oneline --decorate actual 
@@ -447,8 +443,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --decorate=full actual 
test_cmp expect.full actual 
 
-   git config --unset-all log.decorate 
-   git config log.decorate 1 
+   test_config log.decorate 1 
git log --oneline actual 
test_cmp expect.short actual 
git log --oneline --decorate=full actual 
@@ -456,8 +451,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --decorate=no actual 
test_cmp expect.none actual 
 
-   git config --unset-all log.decorate 
-   git config log.decorate short 
+   test_config log.decorate short 
git log --oneline actual 
test_cmp expect.short actual 
git log --oneline --no-decorate actual 
@@ -465,8 +459,7 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --decorate=full actual 
test_cmp expect.full actual 
 
-   git config --unset-all log.decorate 
-   git config log.decorate full 
+   test_config log.decorate full 
git log --oneline actual 
test_cmp expect.full actual 
git log --oneline --no-decorate actual 
@@ -474,16 +467,15 @@ test_expect_success 'log.decorate configuration' '
git log --oneline --decorate actual 
test_cmp expect.short actual
 
-   git config --unset-all log.decorate 
+   test_unconfig log.decorate 
git log --pretty=raw expect.raw 
-   git config log.decorate full 
+   test_config log.decorate full 
git log --pretty=raw actual 
test_cmp expect.raw actual
 
 '
 
 test_expect_success 'reflog is expected format' '
-   test_might_fail git config --remove-section log 
git log -g --abbrev-commit --pretty=oneline expect 
git reflog actual 
test_cmp expect actual
@@ -496,10 +488,6 @@ test_expect_success 'whatchanged is expected format' '
 '
 
 test_expect_success 'log.abbrevCommit configuration' '
-   test_when_finished git config --unset log.abbrevCommit 
-
-   test_might_fail git config --unset log.abbrevCommit 
-
git log --abbrev-commit expect.log.abbrev 
git log --no-abbrev-commit expect.log.full 
git log --pretty=raw expect.log.raw 
@@ -508,7 +496,7 @@ test_expect_success 'log.abbrevCommit configuration' '
git whatchanged --abbrev-commit expect.whatchanged.abbrev 
git whatchanged --no-abbrev-commit expect.whatchanged.full 
 
-   git config log.abbrevCommit true 
+   test_config log.abbrevCommit true 
 
git log actual 
test_cmp expect.log.abbrev actual 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 12/15] t7508: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7508-status.sh | 46 --
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index aecb4d1..e2ffdac 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -131,8 +131,7 @@ cat expect \EOF
 EOF
 
 test_expect_success 'status (advice.statusHints false)' '
-   test_when_finished git config --unset advice.statusHints 
-   git config advice.statusHints false 
+   test_config advice.statusHints false 
git status output 
test_i18ncmp expect output
 
@@ -332,8 +331,7 @@ test_expect_success 'status -uno' '
 '
 
 test_expect_success 'status (status.showUntrackedFiles no)' '
-   git config status.showuntrackedfiles no
-   test_when_finished git config --unset status.showuntrackedfiles 
+   test_config status.showuntrackedfiles no 
git status output 
test_i18ncmp expect output
 '
@@ -348,12 +346,11 @@ cat expect EOF
 #
 # Untracked files not listed
 EOF
-git config advice.statusHints false
 test_expect_success 'status -uno (advice.statusHints false)' '
+   test_config advice.statusHints false 
git status -uno output 
test_i18ncmp expect output
 '
-git config --unset advice.statusHints
 
 cat expect  EOF
  M dir1/modified
@@ -400,8 +397,7 @@ test_expect_success 'status -unormal' '
 '
 
 test_expect_success 'status (status.showUntrackedFiles normal)' '
-   git config status.showuntrackedfiles normal
-   test_when_finished git config --unset status.showuntrackedfiles 
+   test_config status.showuntrackedfiles normal
git status output 
test_i18ncmp expect output
 '
@@ -459,8 +455,7 @@ test_expect_success 'status -uall' '
 '
 
 test_expect_success 'status (status.showUntrackedFiles all)' '
-   git config status.showuntrackedfiles all
-   test_when_finished git config --unset status.showuntrackedfiles 
+   test_config status.showuntrackedfiles all
git status output 
test_i18ncmp expect output
 '
@@ -485,10 +480,9 @@ test_expect_success 'status -s -uall' '
test_cmp expect output
 '
 test_expect_success 'status -s (status.showUntrackedFiles all)' '
-   git config status.showuntrackedfiles all
+   test_config status.showuntrackedfiles all 
git status -s output 
rm -rf dir3 
-   git config --unset status.showuntrackedfiles 
test_cmp expect output
 '
 
@@ -588,15 +582,13 @@ cat expect \EOF
 EOF
 
 test_expect_success 'status with color.ui' '
-   git config color.ui always 
-   test_when_finished git config --unset color.ui 
+   test_config color.ui always 
git status | test_decode_color output 
test_i18ncmp expect output
 '
 
 test_expect_success 'status with color.status' '
-   git config color.status always 
-   test_when_finished git config --unset color.status 
+   test_config color.status always 
git status | test_decode_color output 
test_i18ncmp expect output
 '
@@ -720,8 +712,7 @@ EOF
 
 test_expect_success 'status without relative paths' '
 
-   git config status.relativePaths false 
-   test_when_finished git config --unset status.relativePaths 
+   test_config status.relativePaths false 
(cd dir1  git status) output 
test_i18ncmp expect output
 
@@ -740,8 +731,7 @@ EOF
 
 test_expect_success 'status -s without relative paths' '
 
-   git config status.relativePaths false 
-   test_when_finished git config --unset status.relativePaths 
+   test_config status.relativePaths false 
(cd dir1  git status -s) output 
test_cmp expect output
 
@@ -1038,15 +1028,14 @@ test_expect_success '--ignore-submodules=untracked 
suppresses submodules with un
 '
 
 test_expect_success '.gitmodules ignore=untracked suppresses submodules with 
untracked content' '
-   git config diff.ignoreSubmodules dirty 
+   test_config diff.ignoreSubmodules dirty 
git status output 
test_i18ncmp expect output 
git config --add -f .gitmodules submodule.subname.ignore untracked 
git config --add -f .gitmodules submodule.subname.path sm 
git status output 
test_i18ncmp expect output 
-   git config -f .gitmodules  --remove-section submodule.subname 
-   git config --unset diff.ignoreSubmodules
+   git config -f .gitmodules  --remove-section submodule.subname
 '
 
 test_expect_success '.git/config ignore=untracked suppresses submodules with 
untracked content' '
@@ -1066,15 +1055,14 @@ test_expect_success '--ignore-submodules=dirty 
suppresses submodules with untrac
 '
 
 test_expect_success '.gitmodules ignore=dirty suppresses submodules with 
untracked 

Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

2013-03-24 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 Junio C Hamano wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit 
 or stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

 You should always trust but verify what other commands do at key
 points of the operation; and I think this require-clean-work-tree
 is a key precondition for this mode of operation to work correctly.

 Especially because you do not even bother to check the result of
 git stash before continuing ;-).

 If you think it's enough to replicate the codepath that precedes the
 actual saving in 'git stash' (which is essentially
 require-clean-work-tree), I'm in agreement with you.  I thought you
 were implying a wider safety net, that wouldn't even assume the basic
 sanity of stash.

Er, s/codepath that precedes the actual saving in 'git stash'/codepath
that precedes the actual pulling or merging in 'git pull'/

I'm feeling a little muddled up today; weekends are usually bad.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] contrib/subtree: remove use of -a/-o in [ commands

2013-03-24 Thread Simon Ruderich
From: Paul Campbell pcampb...@kemitix.net

Use of -a and -o in the [ command can have confusing semantics.

Use a separate test invocation for each single test, combining them with
 and ||.

Signed-off-by: Paul Campbell pcampb...@kemitix.net
Signed-off-by: Simon Ruderich si...@ruderich.org
---

On Sun, Mar 24, 2013 at 07:37:42PM +, Paul Campbell wrote:
 Use a separate test invocation for each single test, combining them with
  and ||, and use ordinary parentheses for grouping.

Hello Paul,

Parentheses are only necessary if both  and || are used to
enforce precedence; the shell can split the commands without
needing the parentheses. In these cases they can all be removed.

Regards
Simon

 contrib/subtree/git-subtree.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 5701376..d02e6c5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -119,7 +119,7 @@ esac
 
 dir=$(dirname $prefix/.)
 
-if [ $command != pull -a $command != add -a $command != push ]; 
then
+if test $command != pull  test $command != add  test $command != 
push; then
revs=$(git rev-parse $default --revs-only $@) || exit $?
dirs=$(git rev-parse --no-revs --no-flags $@) || exit $?
if [ -n $dirs ]; then
@@ -181,9 +181,9 @@ cache_set()
 {
oldrev=$1
newrev=$2
-   if [ $oldrev != latest_old \
--a $oldrev != latest_new \
--a -e $cachedir/$oldrev ]; then
+   if test $oldrev != latest_old \
+ test $oldrev != latest_new \
+ test -e $cachedir/$oldrev; then
die cache for $oldrev already exists!
fi
echo $newrev $cachedir/$oldrev
@@ -273,12 +273,12 @@ find_existing_splits()
git-subtree-split:) sub=$b ;;
END)
debug   Main is: '$main'
-   if [ -z $main -a -n $sub ]; then
+   if test -z $main  test -n $sub; then
# squash commits refer to a subtree
debug   Squash: $sq from $sub
cache_set $sq $sub
fi
-   if [ -n $main -a -n $sub ]; then
+   if test -n $main  test -n $sub; then
debug   Prior: $main - $sub
cache_set $main $sub
cache_set $sub $sub
@@ -541,7 +541,7 @@ cmd_add_commit()
tree=$(git write-tree) || exit $?

headrev=$(git rev-parse HEAD) || exit $?
-   if [ -n $headrev -a $headrev != $rev ]; then
+   if test -n $headrev  test $headrev != $rev; then
headp=-p $headrev
else
headp=
-- 
1.8.2

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 04/23] contrib/subtree: Teach push and pull to use .gittrees for defaults

2013-03-24 Thread Paul Campbell
On Mon, Mar 11, 2013 at 3:35 AM, Junio C Hamano gits...@pobox.com wrote:
 From: bibendi bibe...@bk.ru

 Look in the config file .gittrees for a default repository and
 refspec or commit when they are not provided on the command line.

 Uses the .gittrees config file in a similar way to how git-submodule
 uses the .gitmodules file.

 What the patch does can be read from the code, but what benefit
 would users get by the extra file?

How about:

Usually push and pull are to the same repository/branch that they were
originally added from. Add stores the repository/branch in .gittrees
which push and pull can now default to if not provided on the command
line.

  cmd_pull()
  {
 - ensure_clean
 - git fetch $@ || exit $?
 - revs=FETCH_HEAD
 - set -- $revs
 - cmd_merge $@
 +if [ $# -ne 1 ]; then

 Broken indentation?

 + die You must provide branch
 + fi

 It used to allow git fetch $there and let the configured
 remote.$there.fetch refspec to decide what gets fetched, and also it
 used to allow git fetch $there $that_branch to explicitly fetch
 the named branch.  But this change insists that the user has to give
 what gets fetched from the command line and forbids the user from
 giving where to fetch from, it seems.  Isn't it a regression?  Why
 is it a good idea to forbid such uses that the script used to
 accept?

 The proposed log message does not explain why it is not a
 regression, or why accepting some use patterns that the script used
 to allow was a bug that needs to be diagnosed with this new
 conditional.

I think the patch was based on an older version of git-subtree before
git fetch $there support was added. Will need to update it.

 + if [ -e $dir ]; then
 + ensure_clean
 + repository=$(git config -f .gittrees subtree.$prefix.url)
 + refspec=$1
 + git fetch $repository $refspec || exit $?
 + echo git fetch using:  $repository $refspec

 Why are these variable references outside the dq pair?


They're inside now.

Rerolling once I figure out the update for git fetch $there support.

-- 
Paul [W] Campbell
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/15] t7811: remove test_config implementation

2013-03-24 Thread Yann Droneaud
test_config is provided by the test library,
a private version is not needed.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7811-grep-open.sh | 5 -
 1 file changed, 5 deletions(-)

diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh
index a895778..e1951a5 100755
--- a/t/t7811-grep-open.sh
+++ b/t/t7811-grep-open.sh
@@ -125,11 +125,6 @@ test_expect_success 'modified file' '
test_cmp empty out
 '
 
-test_config() {
-   git config $1 $2 
-   test_when_finished git config --unset $1
-}
-
 test_expect_success 'copes with color settings' '
rm -f actual 
echo grep.h expect 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 09/15] t5541: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t5541-http-push.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 4b4b4a6..4086f02 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -181,8 +181,7 @@ test_expect_success 'push (chunked)' '
git checkout master 
test_commit commit path3 
HEAD=$(git rev-parse --verify HEAD) 
-   git config http.postbuffer 4 
-   test_when_finished git config --unset http.postbuffer 
+   test_config http.postbuffer 4 
git push -v -v origin $BRANCH 2err 
grep POST git-receive-pack (chunked) err 
(cd $HTTPD_DOCUMENT_ROOT_PATH/test_repo.git 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 14/15] t9500: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t9500-gitweb-standalone-no-errors.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t9500-gitweb-standalone-no-errors.sh 
b/t/t9500-gitweb-standalone-no-errors.sh
index 90bb605..6783c14 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -539,8 +539,7 @@ test_expect_success \
 test_when_finished GIT_COMMITTER_NAME=\C O Mitter\ 
 echo ISO-8859-1  file 
 git add file 
-git config i18n.commitencoding ISO-8859-1 
-test_when_finished git config --unset i18n.commitencoding 
+test_config i18n.commitencoding ISO-8859-1 
 git commit -F $TEST_DIRECTORY/t3900/ISO8859-1.txt 
 gitweb_run p=.git;a=commit'
 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 05/15] t4304: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Tests are modified to assume correct (default) configuration at entry,
and to reset the modified configuration variables at the end.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t3404-rebase-interactive.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 15dcbd4..a58406d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -937,8 +937,7 @@ test_expect_success 'rebase --edit-todo can be used to 
modify todo' '
 test_expect_success 'rebase -i respects core.commentchar' '
git reset --hard 
git checkout E^0 
-   git config core.commentchar \\ 
-   test_when_finished git config --unset core.commentchar 
+   test_config core.commentchar \\ 
write_script remove-all-but-first.sh -\EOF 
sed -e 2,\$s/^// $1 $1.tmp 
mv $1.tmp $1
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 11/15] t7502: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7502-commit.sh | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index f9b44b7..619d438 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -171,10 +171,9 @@ test_expect_success 'verbose' '
 
 test_expect_success 'verbose respects diff config' '
 
-   git config color.diff always 
+   test_config color.diff always 
git status -v actual 
-   grep \[1mdiff --git actual 
-   git config --unset color.diff
+   grep \[1mdiff --git actual
 '
 
 mesg_with_comment_and_newlines='
@@ -534,8 +533,7 @@ use_template=-t template
 try_commit_status_combo
 
 test_expect_success 'commit --status with custom comment character' '
-   test_when_finished git config --unset core.commentchar 
-   git config core.commentchar ; 
+   test_config core.commentchar ; 
try_commit --status 
test_i18ngrep ^; Changes to be committed: .git/COMMIT_EDITMSG
 '
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/15] t7500: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7500-commit.sh | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 1c908f4..436b7b6 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -36,8 +36,7 @@ test_expect_success 'nonexistent template file should return 
error' '
 '
 
 test_expect_success 'nonexistent template file in config should return error' '
-   git config commit.template $PWD/notexist 
-   test_when_finished git config --unset commit.template 
+   test_config commit.template $PWD/notexist 
(
GIT_EDITOR=echo hello \\$1\ 
export GIT_EDITOR 
@@ -93,14 +92,13 @@ test_expect_success '-t option should be short for 
--template' '
 
 test_expect_success 'config-specified template should commit' '
echo new template  $TEMPLATE 
-   git config commit.template $TEMPLATE 
+   test_config commit.template $TEMPLATE 
echo more content  foo 
git add foo 
(
test_set_editor $TEST_DIRECTORY/t7500/add-content 
git commit
) 
-   git config --unset commit.template 
commit_msg_is new templatecommit message
 '
 
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/15] t4034: use test_config/test_unconfig to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Additionally, instead of
 git config key 
or
 git config --unset key
uses
 test_unconfig key
The latter doesn't failed if key is not defined.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t4034-diff-words.sh | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 40ab333..f2f55fc 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -230,7 +230,7 @@ test_expect_success '.gitattributes override config' '
 '
 
 test_expect_success 'setup: remove diff driver regex' '
-   test_might_fail git config --unset diff.testdriver.wordRegex
+   test_unconfig diff.testdriver.wordRegex
 '
 
 test_expect_success 'use configured regex' '
@@ -335,8 +335,7 @@ test_expect_success 'word-diff with diff.sbe' '
 
c
EOF
-   test_when_finished git config --unset diff.suppress-blank-empty 
-   git config diff.suppress-blank-empty true 
+   test_config diff.suppress-blank-empty true 
word_diff --word-diff=plain
 '
 
@@ -368,7 +367,7 @@ test_expect_success 'setup history with two files' '
 
 test_expect_success 'wordRegex for the first file does not apply to the 
second' '
echo *.tex diff=tex .gitattributes 
-   git config diff.tex.wordRegex [a-z]+|. 
+   test_config diff.tex.wordRegex [a-z]+|. 
cat expect -\EOF 
diff --git a/a.tex b/a.tex
--- a/a.tex
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/15] t5520: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t5520-pull.sh | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 35304b4..cb1a4c5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -96,8 +96,7 @@ test_expect_success '--rebase' '
 '
 test_expect_success 'pull.rebase' '
git reset --hard before-rebase 
-   git config --bool pull.rebase true 
-   test_when_finished git config --unset pull.rebase 
+   test_config pull.rebase true 
git pull . copy 
test $(git rev-parse HEAD^) = $(git rev-parse copy) 
test new = $(git show HEAD:file2)
@@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase' '
git reset --hard before-rebase 
-   git config --bool branch.to-rebase.rebase true 
-   test_when_finished git config --unset branch.to-rebase.rebase 
+   test_config branch.to-rebase.rebase true 
git pull . copy 
test $(git rev-parse HEAD^) = $(git rev-parse copy) 
test new = $(git show HEAD:file2)
@@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' '
 
 test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
git reset --hard before-rebase 
-   git config --bool pull.rebase true 
-   test_when_finished git config --unset pull.rebase 
-   git config --bool branch.to-rebase.rebase false 
-   test_when_finished git config --unset branch.to-rebase.rebase 
+   test_config pull.rebase true 
+   test_config branch.to-rebase.rebase false 
git pull . copy 
test $(git rev-parse HEAD^) != $(git rev-parse copy) 
test new = $(git show HEAD:file2)
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 15/15] t7502: remove clear_config

2013-03-24 Thread Yann Droneaud
Using test_config ensure the configuration variable are removed
at the end of the test, there's no need to remove variable
at the beginning of the test.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7502-commit.sh | 32 ++--
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 619d438..256137f 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -434,16 +434,6 @@ EOF
 
 echo '## Custom template' template
 
-clear_config () {
-   (
-   git config --unset-all $1
-   case $? in
-   0|5)exit 0 ;;
-   *)  exit 1 ;;
-   esac
-   )
-}
-
 try_commit () {
git reset --hard 
echo negative 
@@ -459,67 +449,57 @@ try_commit () {
 try_commit_status_combo () {
 
test_expect_success 'commit' '
-   clear_config commit.status 
try_commit  
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit' '
-   clear_config commit.status 
try_commit  
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --status' '
-   clear_config commit.status 
try_commit --status 
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --no-status' '
-   clear_config commit.status 
try_commit --no-status 
test_i18ngrep ! ^# Changes to be committed: 
.git/COMMIT_EDITMSG
'
 
test_expect_success 'commit with commit.status = yes' '
-   clear_config commit.status 
-   git config commit.status yes 
+   test_config commit.status yes 
try_commit  
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit with commit.status = no' '
-   clear_config commit.status 
-   git config commit.status no 
+   test_config commit.status no 
try_commit  
test_i18ngrep ! ^# Changes to be committed: 
.git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --status with commit.status = yes' '
-   clear_config commit.status 
-   git config commit.status yes 
+   test_config commit.status yes 
try_commit --status 
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --no-status with commit.status = yes' '
-   clear_config commit.status 
-   git config commit.status yes 
+   test_config commit.status yes 
try_commit --no-status 
test_i18ngrep ! ^# Changes to be committed: 
.git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --status with commit.status = no' '
-   clear_config commit.status 
-   git config commit.status no 
+   test_config commit.status no 
try_commit --status 
test_i18ngrep ^# Changes to be committed: .git/COMMIT_EDITMSG
'
 
test_expect_success 'commit --no-status with commit.status = no' '
-   clear_config commit.status 
-   git config commit.status no 
+   test_config commit.status no 
try_commit --no-status 
test_i18ngrep ! ^# Changes to be committed: 
.git/COMMIT_EDITMSG
'
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 13/15] t7600: use test_config to set/unset git config variables

2013-03-24 Thread Yann Droneaud
Instead of using construct such as:
test_when_finished git config --unset key
git config key value
uses
test_config key value
The latter takes care of removing key at the end of the test.

Tests are modified to assume default configuration at entry,
and to reset the modified configuration variables at the end.

Test 'merge log message' was relying on the presence of option `--no-ff`
in the configuration. With the option, git show -s --pretty=format:%b HEAD
produces an empty line and without the option, it produces an empty file.
The test is modified to check with and without `--no-ff` option.

Signed-off-by: Yann Droneaud ydrone...@opteya.com
---
 t/t7600-merge.sh | 60 
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5e19598..2f70433 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -56,7 +56,8 @@ create_merge_msgs () {
echo 
git log --no-merges ^HEAD c2 c3
} squash.1-5-9 
-   echo msg.nolog 
+   : msg.nologff 
+   echo msg.nolognoff 
{
echo * tag 'c3': 
echo   commit 3 
@@ -244,8 +245,7 @@ test_expect_success 'merges with --ff-only' '
 test_expect_success 'merges with merge.ff=only' '
git reset --hard c1 
test_tick 
-   test_when_finished git config --unset merge.ff 
-   git config merge.ff only 
+   test_config merge.ff only 
test_must_fail git merge c2 
test_must_fail git merge c3 
test_must_fail git merge c2 c3 
@@ -336,7 +336,7 @@ test_debug 'git log --graph --decorate --oneline --all'
 
 test_expect_success 'merge c1 with c2 (no-commit in config)' '
git reset --hard c1 
-   git config branch.master.mergeoptions --no-commit 
+   test_config branch.master.mergeoptions --no-commit 
git merge c2 
verify_merge file result.1-5 
verify_head $c1 
@@ -346,12 +346,11 @@ test_expect_success 'merge c1 with c2 (no-commit in 
config)' '
 test_debug 'git log --graph --decorate --oneline --all'
 
 test_expect_success 'merge c1 with c2 (log in config)' '
-   git config branch.master.mergeoptions  
git reset --hard c1 
git merge --log c2 
git show -s --pretty=tformat:%s%n%b expect 
 
-   git config branch.master.mergeoptions --log 
+   test_config branch.master.mergeoptions --log 
git reset --hard c1 
git merge c2 
git show -s --pretty=tformat:%s%n%b actual 
@@ -360,17 +359,12 @@ test_expect_success 'merge c1 with c2 (log in config)' '
 '
 
 test_expect_success 'merge c1 with c2 (log in config gets overridden)' '
-   test_when_finished git config --remove-section branch.master 
-   test_when_finished git config --remove-section merge 
-   test_might_fail git config --remove-section branch.master 
-   test_might_fail git config --remove-section merge 
-
git reset --hard c1 
git merge c2 
git show -s --pretty=tformat:%s%n%b expect 
 
-   git config branch.master.mergeoptions --no-log 
-   git config merge.log true 
+   test_config branch.master.mergeoptions --no-log 
+   test_config merge.log true 
git reset --hard c1 
git merge c2 
git show -s --pretty=tformat:%s%n%b actual 
@@ -380,7 +374,7 @@ test_expect_success 'merge c1 with c2 (log in config gets 
overridden)' '
 
 test_expect_success 'merge c1 with c2 (squash in config)' '
git reset --hard c1 
-   git config branch.master.mergeoptions --squash 
+   test_config branch.master.mergeoptions --squash 
git merge c2 
verify_merge file result.1-5 
verify_head $c1 
@@ -392,7 +386,7 @@ test_debug 'git log --graph --decorate --oneline --all'
 
 test_expect_success 'override config option -n with --summary' '
git reset --hard c1 
-   git config branch.master.mergeoptions -n 
+   test_config branch.master.mergeoptions -n 
test_tick 
git merge --summary c2 diffstat.txt 
verify_merge file result.1-5 msg.1-5 
@@ -406,7 +400,7 @@ test_expect_success 'override config option -n with 
--summary' '
 
 test_expect_success 'override config option -n with --stat' '
git reset --hard c1 
-   git config branch.master.mergeoptions -n 
+   test_config branch.master.mergeoptions -n 
test_tick 
git merge --stat c2 diffstat.txt 
verify_merge file result.1-5 msg.1-5 
@@ -422,7 +416,7 @@ test_debug 'git log --graph --decorate --oneline --all'
 
 test_expect_success 'override config option --stat' '
git reset --hard c1 
-   git config branch.master.mergeoptions --stat 
+   test_config branch.master.mergeoptions --stat 
test_tick 
git merge -n c2 diffstat.txt 
verify_merge file result.1-5 msg.1-5 
@@ -438,7 +432,7 @@ test_debug 'git log --graph --decorate --oneline --all'
 
 

Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks

2013-03-24 Thread David Aguilar
On Sun, Mar 24, 2013 at 5:36 AM, John Keeping j...@keeping.me.uk wrote:
 On Sat, Mar 23, 2013 at 10:19:36PM -0700, Junio C Hamano wrote:
  In the longer term, difftool probably needs to learn to warn the user
  instead of overwrite any changes that have been made to the working tree
  file.

 Questionable.

 Admittedly I do not use difftool myself, and I have long assumed
 that difftool users are using the tools to _view_ the changes, but
 apparently some of the tools let the user muck with what is shown,
 and also apparently people seem to like the fact that they can make
 changes.  So I've led to believe the update in difftool, take the
 change back to working tree, either by making symbolic links or
 copying them back behaviour was a _feature_.

 Yes it is.  I think my explanation wasn't clear enough here.

 What currently happens is that after the user's tool has finished
 running the working tree file and temporary file are compared and if
 they are different then the temporary file is copied over the working
 tree file.

 This is good if the user has edited the temporary file, but what if they
 edit they working tree file while using the tool to examine the
 differences?  I think we need to at the very least look at the mtime of
 the files and refuse to copy over the temporary file if that of the
 working tree file is newer.

 Obviously none of this matters if we can use symlinks, but in the
 non-symlink case I think a user might find it surprising if the
 (unmodified) file used by their diff tool were suddenly copied over the
 working tree wiping out the changes they have just made.

Thanks, this adds a little more safety to the operation, which is good.
The downside is that it's a performance hit since we end up running
an additional hash-object on every worktree file.
I would definitely choose safety/correctness in this situation.

This makes me wonder whether the modifiable mode should be made
more explicit, either in the documentation or via a flag.

Imagine if --dir-diff also honored --edit and --no-edit flags.

Right now --edit is the default.  If we had foreseen these various
edge cases and unintended copy-backs then we may have initially
chosen --no-edit as the default, but that's not really my point.

What I'm thinking is that it might be good for the tool to
learn --edit/--no-edit so that the symlink/copy-back heuristic
can be documented alongside that option.  Users can then know
what to expect when using this mode.  --no-edit would also be
faster since it can avoid all these extra steps.

It could also learn difftool.dirDiffEditable to control the
default, which would eliminate the pain in needing to supply
the flag on every invocation.

What do you think about officially supporting a read-only mode?
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-p4: support exclusively locked files

2013-03-24 Thread Pete Wyckoff
danny.tho...@blackboard.com wrote on Wed, 20 Mar 2013 07:33 -0400:
 Sounds good to me. I've got a couple of busy days coming up, but should
 have time this week.

Here's what I'm playing with for test cases, by the way.  The fix
you're working on is definitely part of it, but there are more
issues as well.  I'll address them once you've taken care of the
opened/fstat issue.  Thanks,

-- Pete

--- 8 ---

From c6691126ae75c364763ab4d774c75045285b8ddd Mon Sep 17 00:00:00 2001
From: Pete Wyckoff p...@padd.com
Date: Sun, 17 Mar 2013 16:05:07 -0400
Subject: [PATCH] git p4 test: examine behavior with locked (+l) files

The p4 server can enforce file locking, so that only one user
can edit a file at a time.  Git p4 is unable to submit changes
to locked files.  Currently it exits poorly.  Ideally it would
notice the locked condition and clean up nicely.
---
 t/t9816-git-p4-locked.sh | 145 +++
 1 file changed, 145 insertions(+)
 create mode 100755 t/t9816-git-p4-locked.sh

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+# See
+# 
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure sitewide pessimistic locking
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+   (
+   cd $cli 
+   echo TypeMap: +l //depot/... | p4 typemap -i 
+   echo file1 file1 
+   p4 add file1 
+   p4 submit -d add file1
+   )
+'
+
+test_expect_success 'edit with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line2 file1 
+   git add file1 
+   git commit -m line2 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
+test_expect_failure 'add with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line1 add-lock-not-taken 
+   git add file2 
+   git commit -m add add-lock-not-taken 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+lock_in_another_client() {
+   # build a different client
+   cli2=$TRASH_DIRECTORY/cli2 
+   mkdir -p $cli2 
+   test_when_finished p4 client -f -d client2  rm -rf \$cli2\ 
+   (
+   cd $cli2 
+   P4CLIENT=client2 
+   cli=$cli2 
+   client_view //depot/... //client2/... 
+   p4 sync 
+   p4 open file1
+   )
+}
+
+test_expect_failure 'edit with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line3 file1 
+   git add file1 
+   git commit -m line3 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'delete with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git rm file1 
+   git commit -m delete file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'chmod with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   chmod +x file1 
+   git add file1 
+   git commit -m chmod +x file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'copy with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   cp file1 file2 
+   git add file2 
+   git commit -m cp file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectCopies true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'move with lock taken' '
+  

Re: [PATCH 8/4] match-trees: drop x = x initializations

2013-03-24 Thread René Scharfe
Am 24.03.2013 05:55, schrieb Junio C Hamano:
 However, the same compiler still thinks {elem,path,mode}1 can be
 used uninitialized (but not {elem,path,mode}2).  The craziness I
 reported in the previous message is also the same.  With this patch
 on top to swap the side we inspect first, the compiler thinks
 {elem,path,mode}2 can be used uninitialized but not the other three
 variables X-.
 
 So I like your change for readability, but for GCC 4.4.5 we still
 need the unnecessary initialization.

Hrm, perhaps we can make it even simpler for the compiler.

-- 8 --
Subject: match-trees: simplify score_trees() using tree_entry()

Convert the loop in score_trees() to tree_entry().  The code becomes
shorter and simpler because the calls to update_tree_entry() are not
needed any more.

Another benefit is that we need less variables to track the current
tree entries; as a side-effect of that the compiler has an easier
job figuring out the control flow and thus can avoid false warnings
about uninitialized variables.

Using struct name_entry also allows the use of tree_entry_len() for
finding the path length instead of strlen(), which may be slightly
more efficient.

Also unify the handling of missing entries in one of the two trees
(i.e. added or removed files): Just set cmp appropriately first, no
matter if we ran off the end of a tree or if we actually have two
entries to compare, and check its value a bit later without
duplicating the handler code.

Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
I'm a bit uneasy about this one because we lack proper tests for
this code and I don't know how to write ones off the bat.

 match-trees.c | 68 ---
 1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 26f7ed1..2bb734d 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -47,6 +47,13 @@ static int score_matches(unsigned mode1, unsigned mode2, 
const char *path)
return score;
 }
 
+static int base_name_entries_compare(const struct name_entry *a,
+const struct name_entry *b)
+{
+   return base_name_compare(a-path, tree_entry_len(a), a-mode,
+b-path, tree_entry_len(b), b-mode);
+}
+
 /*
  * Inspect two trees, and give a score that tells how similar they are.
  */
@@ -71,54 +78,35 @@ static int score_trees(const unsigned char *hash1, const 
unsigned char *hash2)
if (type != OBJ_TREE)
die(%s is not a tree, sha1_to_hex(hash2));
init_tree_desc(two, two_buf, size);
-   while (one.size | two.size) {
-   const unsigned char *elem1 = elem1;
-   const unsigned char *elem2 = elem2;
-   const char *path1 = path1;
-   const char *path2 = path2;
-   unsigned mode1 = mode1;
-   unsigned mode2 = mode2;
+   for (;;) {
+   struct name_entry e1, e2;
+   int got_entry_from_one = tree_entry(one, e1);
+   int got_entry_from_two = tree_entry(two, e2);
int cmp;
 
-   if (one.size)
-   elem1 = tree_entry_extract(one, path1, mode1);
-   if (two.size)
-   elem2 = tree_entry_extract(two, path2, mode2);
-
-   if (!one.size) {
-   /* two has more entries */
-   score += score_missing(mode2, path2);
-   update_tree_entry(two);
-   continue;
-   }
-   if (!two.size) {
+   if (got_entry_from_one  got_entry_from_two)
+   cmp = base_name_entries_compare(e1, e2);
+   else if (got_entry_from_one)
/* two lacks this entry */
-   score += score_missing(mode1, path1);
-   update_tree_entry(one);
-   continue;
-   }
-   cmp = base_name_compare(path1, strlen(path1), mode1,
-   path2, strlen(path2), mode2);
-   if (cmp  0) {
+   cmp = -1;
+   else if (got_entry_from_two)
+   /* two has more entries */
+   cmp = 1;
+   else
+   break;
+
+   if (cmp  0)
/* path1 does not appear in two */
-   score += score_missing(mode1, path1);
-   update_tree_entry(one);
-   continue;
-   }
-   else if (cmp  0) {
+   score += score_missing(e1.mode, e1.path);
+   else if (cmp  0)
/* path2 does not appear in one */
-   score += score_missing(mode2, path2);
-   update_tree_entry(two);
-   continue;
-   }
-

Re: [PATCH] checkout: add --sparse for restoring files in sparse checkout mode

2013-03-24 Thread Duy Nguyen
On Mon, Mar 25, 2013 at 1:17 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 +--sparse::
 + In sparse checkout mode, `git checkout -- paths` would
 + update all entries matched by paths regardless sparse
 + patterns. This option only updates entries matched by paths
 + and sparse patterns.

 Hm, should this be the default?

 In principle, I would expect

 git checkout -- .

 to make the worktree match the index, respecting the sparse checkout.
 And something like

 git checkout --widen -- .

 to change the sparse checkout pattern.  But of course it is easily
 possible that I am missing some details of how sparse checkout is
 used in practice.

 What do you think?

Changing the default may involve a painful transition phase (e.g. add
-u). I think making it the default via alias should be good enough in
most cases. We also need to think how it impacts checkout usage in
scripts. I think it might be ok, but I haven't finished my morning
coffee yet, so..
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] checkout: add --sparse for restoring files in sparse checkout mode

2013-03-24 Thread Jonathan Nieder
Duy Nguyen wrote:
 On Mon, Mar 25, 2013 at 1:17 AM, Jonathan Nieder jrnie...@gmail.com wrote:

 Hm, should this be the default?

 In principle, I would expect

 git checkout -- .

 to make the worktree match the index, respecting the sparse checkout.
 And something like

 git checkout --widen -- .

 to change the sparse checkout pattern.
[...]
 Changing the default may involve a painful transition phase (e.g. add
 -u).

I don't think it needs to.  There aren't many people using sparse
checkout even these days, and I think they'd generally be happy about
the change.

But if we want to be conservative until some later point (like 2.1),
perhaps --sparse should be named something like --no-widen?  That
way, I can do

git checkout --no-widen -- .

to make the worktree match the index, respecting the sparse checkout.
And I can do

git checkout --widen -- .

to change the sparse checkout pattern.  Meanwhile the confusing
command

git checkout -- .

would be ill-defined for sparse checkouts --- in past git versions,
if I understand you correctly it acted like --widen, while in some
unspecified future version it may change to mean --no-widen.  No
need for warnings because I doubt anyone is relying on either
behavior.

Would that work?
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html