Git development
 help / color / mirror / Atom feed
* [PATCH 1/2] git-p4: remove bash-ism in t9809
From: Pete Wyckoff @ 2012-02-26 15:37 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes
In-Reply-To: <1330270647-8817-1-git-send-email-pw@padd.com>

Plain old $# works to count the number of arguments in
either bash or dash, even if the arguments have spaces.

Based-on-patch-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9809-git-p4-client-view.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index ae9145e..18d93e4 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -31,7 +31,7 @@ client_view() {
 #
 check_files_exist() {
 	ok=0 &&
-	num=${#@} &&
+	num=$# &&
 	for arg ; do
 		test_path_is_file "$arg" &&
 		ok=$(($ok + 1))
-- 
1.7.9.2.288.g74b75

^ permalink raw reply related

* [PATCH 0/2] git-p4: remove test bash-isms
From: Pete Wyckoff @ 2012-02-26 15:37 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

Two fixes to make the t98* tests run in dash.

Pete Wyckoff (2):
  git-p4: remove bash-ism in t9809
  git-p4: remove bash-ism in t9800

 t/t9800-git-p4-basic.sh       |   25 ++++++++++++++++---------
 t/t9809-git-p4-client-view.sh |    2 +-
 2 files changed, 17 insertions(+), 10 deletions(-)

-- 
1.7.9.2.288.g74b75

^ permalink raw reply

* Re: Question about your comment on the git parable
From: Jakub Narebski @ 2012-02-26 15:06 UTC (permalink / raw)
  To: Federico Galassi; +Cc: git
In-Reply-To: <1E5ECB5A-595A-4B04-8269-6E35BF3FEA1A@gmail.com>

On Sun, 26 Feb 2012, Federico Galassi wrote:
> On 26/feb/2012, at 12:29, Jakub Narebski wrote:
> 
>> Would you mind if this discussion was moved to git mailing
>> list (git@vger.kernel.org), of course always with copy directly
>> to you?  There are people there that can answer your questions
>> better.
> 
> No problem.
>
>> On Sun, 26 Feb 2012, Federico Galassi wrote:
>>> Hello, i think you're the author of these comments:
>>> http://news.ycombinator.com/item?id=616610 
>>> 
>>> I'm doing educational work on git based on the parable (talks,
>>> articles, etc..) and i'd like to improve on the real reason
>>> for a staging area.  
>>> 
>>> My question basically is: why is it really needed for merging?
>>> I mean, given the fictional git-like system of the parable,
>>> if I need to merge 2 snapshots i could: 
>>> 
>>> 1) search the commit tree for a base point
[...]
>>> 2) compare the diffs between the snapshots and the base point snapshot
>>> 3) if a conflict happens (change in the same line), just leave
>>>   something in the working dir to mark the conflict. For example,
>>>   keeping it simple, the system could reject a new commit until
>>>   the markers of the conflict are removed from the conflicting file.   
>>> 
>>> Couldn't it just work this way?
>> 
>> Well, it could; that is how many if not most of other version control
>> systems work.
>> 
>> 
>> There are (at least!) three problems with that approach.  First, sometimes
>> it is not possible to "leave something in the working dir to mark the
>> conflict".  Take for example case where binary file (e.g. image) was
>> changed, and textual 3-way diff file-merge algorithm wouldn't work.
>> 
>> Second, what to do in the case of *tree-level* conflict, for example
>> rename/rename conflict, where one side renamed file to different
>> name (moved to different place) than the other side.  There are no
>> conflict markers for this...
>> 
>> Third, what about false positives with detecting conflict markers,
>> i.e. the case where "rejecting new commit until conflict markers are
>> removed", for example AsciiDoc files can be falsely detected as having
>> partial conflict markers, and of course test vectors for testing conflict
>> would have to have conflict markers in them.
> 
> Ok, it's clear to me that the markers in file approach is just a little
> bit too simple. Do you see any concrete advantage in the staging area
> compared to, say, tree conflict metadata in the working dir and maybe
> a dedicated smart "resolve conflict" command?   

First, for such _local_ information working directory isn't the best place.
What if you accidentally delete this?  It is not and should not be
committed to repository,so there is no way to undelete it, except redoing
merge and losing all your progress so far in resolving merge conflicts.
It is much better to put such information somewhere in administrative
area[1] of repository. 

Second, if we have staging area where we store information about which
files are tracked, and a bunch of per-file metadata like modification time
for better performance, why not use it also for storing information about
merge in progress?

[1]: Name taken from "Version Control by Example" (free e-book) by
     Eric Sink.


There is also a thing very specific to Git, namely that "git add" adds
a current content of a file to object database of a repository (though
with modern git there is also "git add --intent-to-add" which works 
like add-ing file in other version control systems)... and you have to
store reference to newly created object somewhere so that it doesn't get
garbage-collected.

>>> Can you mention other situations in which the pattern "files to be added"
>>> is either mandatory or really helpful? 
>> 
>> Note that any version control system must have a kind of proto-staging
>> area to know which files are to be added in next commit.
>> 
>> If you do
>> 
>>  $ scm add file.c
>> 
>> then version control system must save somewhere that 'file.c' is to be
>> tracked (to be added in next commit).
> 
> Yes, the fictional vcs just tracked all the files in the working dir.
> Being selective on which file to track is of course another interesting
> feature.  

IRL it is a _necessary_ feature.  One of more common, if not most common
application of version control system is to manage source files for a
computer program.  And there you have object files, executables and other
_generated_ files which shouldn't be put in version control, not to
mention backups created by your editor / IDE (e.g. "*~" files in Unix
world, "*.bak" files in MS Windows world).

Not to mention files which you have added to working directory, but are
not ready to be added to new commit.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH] git-p4: submit files with wildcards
From: Pete Wyckoff @ 2012-02-26 14:55 UTC (permalink / raw)
  To: git; +Cc: Luke Diamand

There are four wildcard characters in p4.  Files with these
characters can be added to p4 repos using the "-f" option.  They
are stored in %xx notation, and when checked out, p4 converts
them back to normal.

When adding files with wildcards in git, the submit path must
be careful to use the encoded names in some places, and it
must use "-f" to add them.

Support for wildcards in the clone/sync path was added in
084f630 (git-p4: decode p4 wildcard characters, 2011-02-19),
but that change did not handle the submit path.

Reported-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |   64 ++++++++++++++++++++++++++++---------------
 t/t9800-git-p4-basic.sh    |   23 ++++++++++++++++
 2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 0539553..bd89402 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -136,8 +136,8 @@ def p4_integrate(src, dest):
 def p4_sync(path):
     p4_system(["sync", path])
 
-def p4_add(f):
-    p4_system(["add", f])
+def p4_add(path, *options):
+    p4_system(["add"] + list(options) + [path])
 
 def p4_delete(f):
     p4_system(["delete", f])
@@ -232,8 +232,12 @@ def setP4ExecBit(file, mode):
     # Reopens an already open file and changes the execute bit to match
     # the execute bit setting in the passed in mode.
 
-    p4Type = "+x"
+    # Since this is working on already opened files, any with wildcards
+    # in the name have been encoded already, and must be referred to
+    # with the encoded name.
+    file = wildcard_encode(file)
 
+    p4Type = "+x"
     if not isModeExec(mode):
         p4Type = getP4OpenedType(file)
         p4Type = re.sub('^([cku]?)x(.*)', '\\1\\2', p4Type)
@@ -596,6 +600,34 @@ def p4PathStartsWith(path, prefix):
         return path.lower().startswith(prefix.lower())
     return path.startswith(prefix)
 
+#
+# P4 wildcards are not allowed in filenames.  P4 complains
+# if you simply add them, but you can force it with "-f", in
+# which case it translates them into %xx encoding internally.
+#
+def wildcard_decode(path):
+    # Search for and fix just these four characters.  Do % last so
+    # that fixing it does not inadvertently create new %-escapes.
+    # Cannot have * in a filename in windows; untested as to
+    # what p4 would do in such a case.
+    if not platform.system() == "Windows":
+        path = path.replace("%2A", "*")
+    path = path.replace("%23", "#") \
+               .replace("%40", "@") \
+               .replace("%25", "%")
+    return path
+
+def wildcard_encode(path):
+    # do % first to avoid double-encoding the %s introduced here
+    path = path.replace("%", "%25") \
+               .replace("*", "%2A") \
+               .replace("#", "%23") \
+               .replace("@", "%40")
+    return path
+
+def wildcard_present(path):
+    return path.translate(None, "*#@%") != path
+
 class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
@@ -1099,7 +1131,12 @@ class P4Submit(Command, P4UserMap):
         system(applyPatchCmd)
 
         for f in filesToAdd:
-            p4_add(f)
+            # forcibly add file names with wildcards; see also
+            # wildcard_decode() in the sync path
+            if wildcard_present(f):
+                p4_add(f, "-f")
+            else:
+                p4_add(f)
         for f in filesToDelete:
             p4_revert(f)
             p4_delete(f)
@@ -1537,23 +1574,6 @@ class P4Sync(Command, P4UserMap):
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
 
-    #
-    # P4 wildcards are not allowed in filenames.  P4 complains
-    # if you simply add them, but you can force it with "-f", in
-    # which case it translates them into %xx encoding internally.
-    # Search for and fix just these four characters.  Do % last so
-    # that fixing it does not inadvertently create new %-escapes.
-    #
-    def wildcard_decode(self, path):
-        # Cannot have * in a filename in windows; untested as to
-        # what p4 would do in such a case.
-        if not self.isWindows:
-            path = path.replace("%2A", "*")
-        path = path.replace("%23", "#") \
-                   .replace("%40", "@") \
-                   .replace("%25", "%")
-        return path
-
     # Force a checkpoint in fast-import and wait for it to finish
     def checkpoint(self):
         self.gitStream.write("checkpoint\n\n")
@@ -1638,7 +1658,7 @@ class P4Sync(Command, P4UserMap):
 
     def streamOneP4File(self, file, contents):
         relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
-        relPath = self.wildcard_decode(relPath)
+        relPath = wildcard_decode(relPath)
         if verbose:
             sys.stderr.write("%s\n" % relPath)
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 04ee20e..22669df 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -163,6 +163,29 @@ test_expect_success 'wildcard files git-p4 clone' '
 	)
 '
 
+test_expect_success 'wildcard files submit back to p4' '
+	"$GITP4" clone --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		echo git-wild-hash >git-wild#hash &&
+		echo git-wild-star >git-wild\*star &&
+		echo git-wild-at >git-wild@at &&
+		echo git-wild-percent >git-wild%percent &&
+		git add git-wild* &&
+		git commit -m "add some wildcard filenames" &&
+		git config git-p4.skipSubmitEditCheck true &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file git-wild#hash &&
+		test_path_is_file git-wild\*star &&
+		test_path_is_file git-wild@at &&
+		test_path_is_file git-wild%percent
+	)
+'
+
 test_expect_success 'clone bare' '
 	"$GITP4" clone --dest="$git" --bare //depot &&
 	test_when_finished cleanup_git &&
-- 
1.7.9.220.g4b839

^ permalink raw reply related

* Re: Question about your comment on the git parable
From: Jakub Narebski @ 2012-02-26 14:10 UTC (permalink / raw)
  To: Federico Galassi; +Cc: git
In-Reply-To: <4B4C5353-9820-4068-92DA-50665B1011E1@gmail.com>

Federico Galassi wrote:
> On 26/feb/2012, at 13:03, Jakub Narebski wrote:
>> Jakub Narebski wrote:

[...]
>>> Note also that the staging area is also a performance hack (perhaps it
>>> began as such; I am not sure about this aspect of git history).  Git uses
>>> it to be able to _cheaply_ check which files were changed.
>> 
>> The first name for staging area, _dircache_, hints at this.
> 
> Unfortunately, i'm not into git development. Do you have a clue on why
> the index, apparently a tree referring to objects, is much faster than
> reading that stuff right from the database?  

The index (at the very beginning "dircache"), or the staging area, stores
more information that are saved in object database, for example stat
information (file metadata).  Most of file metadata is highly local, so
it doesn't make sense to save it in object database of repository, but
it is used to avoid a file read: usually stat-ing a file, which is much
more cheap, is enough to notice that the file did not change.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH 2/2] index-pack: reduce memory usage when the pack has large blobs
From: Ian Kumlien @ 2012-02-26 13:28 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8Cncs8RYiSB0N20vy9zu2NRTTHpfw3rSfmW64i-4_wxSw@mail.gmail.com>

On Sun, Feb 26, 2012 at 11:10:14AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Sun, Feb 26, 2012 at 5:45 AM, Ian Kumlien <pomac@vapor.com> wrote:
> > Actually, i added a backtrace and used addr2line to confirm my
> > suspicion... which is:
> > builtin/index-pack.c:414
> >
> > ie get_data_from_pack...
> 
> That function should only be called when objects are deltified, which
> should _not_ happen for large blobs. What is its caller?

Full backtrace:

for x in 0x536031 0x451b0e 0x452212 0x4523f5 0x452711 0x452799 0x452bbb
0x454344 0x4170d1 0x41726c ; do addr2line $x -e ../git/git ; done
git/wrapper.c:41
git/builtin/index-pack.c:414
git/builtin/index-pack.c:588
git/builtin/index-pack.c:625
git/builtin/index-pack.c:679
git/builtin/index-pack.c:694
git/builtin/index-pack.c:805
git/builtin/index-pack.c:1246
git/git.c:308
git/git.c:467

Which means:
xmalloc
get_data_from_pack
get_base_data -- line just after: if (!delta_nr) {
resolve_delta
find_unresolved_deltas_1
find_unresolved_deltas
parse_pack_objects
cmd_index_pack
[skipping the git.c part]

Btw, i'm running these tests on a 64 bit laptop - since i'm not at work
;) (had to manually limit xmalloc but it triggers at the same point)

^ permalink raw reply

* Re: sha-1 check in rev-list --verify-objects redundant?
From: Nguyen Thai Ngoc Duy @ 2012-02-26 11:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vk43af14m.fsf@alter.siamese.dyndns.org>

On Sun, Feb 26, 2012 at 3:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> ... I wonder if this is a
>> redundant check. --verify-objects is called to verify new packs.
>
> I do not think --verify-objects does not have anything to do with
> verifying the integrity of packs, whether new or old.
>
> The check is about the integrity of the *history* we _might_ already have
> on our side, when we find ourselves wanting to fetch up to a commit $X,
> whose reachability from the tips of our refs (i.e. the objects that are
> guaranteed to be present in our repository) is unknown, and we somehow
> already have the commit $X itself in the repository.
>
> We cannot just declare victory upon seeing commit $X and omit fetching the
> history leading to the commit, because we may or may not have its parent
> commit object, or the tree object that is recorded in it (it may be that
> we killed an HTTP walker after we fetched $X but not its parents or
> trees).  We need to walk back from $X until we hit one of the tips of our
> refs, and while doing so, we also need to make sure the trees and blobs
> referenced from the walked commits are also healthy.
>
> As 5a48d24 (rev-list --verify-object, 2011-09-01) explains, we used to do
> this with --objects instead, but that check does not even make sure blobs
> exist [*1*] let alone checking to see if these blobs were healthy.  The
> whole point of using --verify-objects instead of --objects is to make sure
> that we do not miss blob objects.

"rev-list --objects" does check for blob existence, in finish_object().

On the well-formedness, unless I'm mistaken, --verify-objects is
_always_ used in conjunction with index-pack. --verify-object is not
even documented. index-pack makes sure all (new) object signatures
reflect their content. Commit and tree content are validated by
rev-list walking them. So at least when --verify-objects is used with
index-pack, I don't see the point rehashing every new object _again_.

> [Footnote]
>
> *1* The --objects code reads the commits and trees in order to _list_
> objects to the blob level, so implicitly, it validates that commits and
> trees reachable from the commit $X we happened to have in our repository,
> relying on the fact that we would error out if we fail to read them.



-- 
Duy

^ permalink raw reply

* Re: sha-1 check in rev-list --verify-objects redundant?
From: Junio C Hamano @ 2012-02-26  9:11 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <7vk43af14m.fsf@alter.siamese.dyndns.org>

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

> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> ... I wonder if this is a
>> redundant check. --verify-objects is called to verify new packs.
>
> I do not think --verify-objects does not have anything to do with
> verifying the integrity of packs, whether new or old.

A typo/grammo that should be obvious from the context, sorry.  The above
should be "I do not think --verify-objects has anything to do with ..."

^ permalink raw reply

* Re: 4-way diff (base,ours,theirs,merged) to review merge results
From: Junio C Hamano @ 2012-02-26  9:05 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jicafn$gnj$1@dough.gmane.org>

"Neal Kreitzinger" <neal@rsss.com> writes:

> ....  What is the 
> best way to display a 4-way diff of merge-base, "ours", "theirs", and 
> "merged" after a merge completes so you can review the "merged" results for 
> correctness?

Ahh, sorry.  While everything I wrote in my previous reply is correct with
respect to what happens _during_ a merge until you resolve it, I did not
realize that you were asking how to view a merge _after_ it is made.

For a two-parent merge $M, "git show --cc $M" runs a three-way diff
between $M (merge result), $M^1 (the first parent) and $M^2 (the other
parent) and the combined diff it shows is equivalent to:

  $ git diff --cc $M $M^1 $M^2

Notice the order of parameters. Unlike a normal "diff A B" to ask the
command to explain how the state B is reached from state A, you give the
result $M and ask the command to explain how it was reached from other
states.

So in a similar way, running

  $ git diff --cc $M $M^1 $M^2 $(git merge-base $M^1 $M^2)

should show a combined patch that explains the state at $M relative to the
states recorded in its parents and the merge base.

I've never tried it myself, though, as I never needed such an operation.

You can try a trivial example with 4d9e079, which merges 583c389 ec7ff5b
and has conflicts in cache.h

$ git show 4d9e079 -- cache.h
Output omitted; you can see it is the same as the next one for yourself.

$ git diff --cc 4d9e079 583c389 ec7ff5b -- cache.h
diff --cc cache.h
index 3a8e125,24732e6..422c5cf
--- a/cache.h
+++ b/cache.h
@@@ -1177,7 -1176,7 +1177,8 @@@ extern void setup_pager(void)
  extern const char *pager_program;
  extern int pager_in_use(void);
  extern int pager_use_color;
 +extern int term_columns(void);
+ extern int decimal_width(int);
  
  extern const char *editor_program;
  extern const char *askpass_program;

One side adds term_columns, the other side adds decimal_width.

$ git diff --cc 4d9e079 583c389 ec7ff5b \
    $(git merge-base 583c389 ec7ff5b) -- cache.h
diff --cc cache.h
index 3a8e125,24732e6,9bd8c2d..422c5cf
--- a/cache.h
+++ b/cache.h
@@@@ -1177,7 -1176,7 -1176,6 +1177,8 @@@@ extern void setup_pager(void)
   extern const char *pager_program;
   extern int pager_in_use(void);
   extern int pager_use_color;
 ++extern int term_columns(void);
+ +extern int decimal_width(int);
   
   extern const char *editor_program;
   extern const char *askpass_program;

The third column is a diff between $M and $(git merge-base $M^1 $M^2); the
resulting two new lines are indeed shown as additions against the merge
base.

^ permalink raw reply

* Re: sha-1 check in rev-list --verify-objects redundant?
From: Junio C Hamano @ 2012-02-26  8:30 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <CACsJy8D_BdV14dGc2YsK91FrX8S=70DJOY3cU=oH3y41N2Ar0w@mail.gmail.com>

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

> ... I wonder if this is a
> redundant check. --verify-objects is called to verify new packs.

I do not think --verify-objects does not have anything to do with
verifying the integrity of packs, whether new or old.

The check is about the integrity of the *history* we _might_ already have
on our side, when we find ourselves wanting to fetch up to a commit $X,
whose reachability from the tips of our refs (i.e. the objects that are
guaranteed to be present in our repository) is unknown, and we somehow
already have the commit $X itself in the repository.

We cannot just declare victory upon seeing commit $X and omit fetching the
history leading to the commit, because we may or may not have its parent
commit object, or the tree object that is recorded in it (it may be that
we killed an HTTP walker after we fetched $X but not its parents or
trees).  We need to walk back from $X until we hit one of the tips of our
refs, and while doing so, we also need to make sure the trees and blobs
referenced from the walked commits are also healthy.

As 5a48d24 (rev-list --verify-object, 2011-09-01) explains, we used to do
this with --objects instead, but that check does not even make sure blobs
exist [*1*] let alone checking to see if these blobs were healthy.  The
whole point of using --verify-objects instead of --objects is to make sure
that we do not miss blob objects.


[Footnote]

*1* The --objects code reads the commits and trees in order to _list_
objects to the blob level, so implicitly, it validates that commits and
trees reachable from the commit $X we happened to have in our repository,
relying on the fact that we would error out if we fail to read them.

^ permalink raw reply

* Re: 4-way diff (base,ours,theirs,merged) to review merge results
From: Junio C Hamano @ 2012-02-26  8:12 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jicafn$gnj$1@dough.gmane.org>

"Neal Kreitzinger" <neal@rsss.com> writes:

> (Combined diff)
> ours:  has line-x
> theirs (master):  does not have line-x
> merged:  has line-x
> merge-base (older master):  *may-or-may-not* have line-x
> conclusion:  I'm not very sure if "merged" should have line-x or not...

When I need this information to resolve a merge in an area of the code
that I am not very familiar with, the first thing I do is this:

  $ git merge $other
  $ git diff
  ... yikes, that is a complex conflict!

  $ git checkout --conflict=diff3 $the_path_with_difficult_conflict
  $ git diff

The output will also show the lines from the merge base.

The default style of showing the conflict we use is called the "merge"
style (it originally came from the "merge" program of the RCS suite), and
it only gives the two sides without the base version.  It is sufficient
when the person who is making the merge is familiar with the baseline
history of the code (e.g. in a contributor-to-integrator pull based
workflow, especially when contributors are encouraged to keep their topics
focused and short). The "diff3" style that also gives the base version is
needed less often in such a setting. That, and also the resulting output
is much shorter, is the reason why "merge" style is the default.

When the person who is making the merge is not very familiar with the
baseline history (e.g. when using Git as an improved CVS and a contributor
pulls the updated upstream into his history), however, "diff3" style may
be more often helpful---as you mentioned, "merge" style requires that you
know your code well enough to either already know or be able to guess how
the version in the merge base looked like, but by definition, pulling the
updated upstream into your work will pull more stuff (because many other
people are working on the code on the other side) than pulling one topic
from a contributor into the integrator tree, so there may be more need to
see the version from the merge base in such a workflow.

By setting the configuration variable "merge.conflictstyle" to "diff3",
you would get the base version by default whenever there is a conflict.

^ permalink raw reply

* Re: [PATCH 2/2] index-pack: reduce memory usage when the pack has large blobs
From: Nguyen Thai Ngoc Duy @ 2012-02-26  4:10 UTC (permalink / raw)
  To: Ian Kumlien; +Cc: git
In-Reply-To: <20120225224533.GJ9526@pomac.netswarm.net>

On Sun, Feb 26, 2012 at 5:45 AM, Ian Kumlien <pomac@vapor.com> wrote:
> Actually, i added a backtrace and used addr2line to confirm my
> suspicion... which is:
> builtin/index-pack.c:414
>
> ie get_data_from_pack...

That function should only be called when objects are deltified, which
should _not_ happen for large blobs. What is its caller?

>
> It looks to me like, if we are to support this kind of things, we need a
> slightly different approach - instead of passing the data around, it
> feels like passing a function pointer around would be beneficial.
>
> Looking at the code i see alot of places where this would be a issue,
> just the fact that get_data_from_pack is used in several functions that
> might do some small operation and then just free it.
>
> I understand and recognize that my "problem" is not what git was
> designed for; it was designed for small files, which is very evident in
> how it approaches the data... And I'd most definetly have to look alot
> closer to this code... =)
>
>> --
>> Duy



-- 
Duy

^ permalink raw reply

* 4-way diff (base,ours,theirs,merged) to review merge results
From: Neal Kreitzinger @ 2012-02-26  3:55 UTC (permalink / raw)
  To: git

Combined diff only tells you what the merge result auto-resolved (with 
rerere turned off and no merge-conflicts) in comparison to "ours" and 
"theirs".  That only tells you what "ours" and "theirs" *had*, not what they 
*did* (or were trying to do).  You need the merge-base version to see what 
"ours" and "theirs" did.  Seeing what "ours" and "theirs" did will 
much-better tell you if "merged" did-the-right-thing or not.  What is the 
best way to display a 4-way diff of merge-base, "ours", "theirs", and 
"merged" after a merge completes so you can review the "merged" results for 
correctness?

Before I try writing a script to dump the object-contents of the merge-base, 
"ours", "theirs", and "merged" versions of the-file-in-question to 
work-files and then feed them to a 4-way diff for review, I would like to 
see if someone already has a script or better-idea for this, or if git has 
something more straight-forward that already does-this-for-you.

Reason for this:
If "ours" has line-x and "theirs" does not have line-x, and "merged" does 
have line-x you still have a mystery on your hands:

(Combined diff)
ours:  has line-x
theirs (master):  does not have line-x
merged:  has line-x
merge-base (older master):  *may-or-may-not* have line-x
conclusion:  I'm not very sure if "merged" should have line-x or not...

Based on the combined-diff only, I don't know if "merged" should have line-x 
or not because I don't know if "ours" *added* line-x to the merge-base or if 
"theirs" *removed* line-x from the merge-base.  IOW, if "theirs" is master 
and "ours" is way-behind master then I pretty-much know I probably need to 
take "theirs" because it has the latest-stuff.  However, I don't know if 
"theirs" took line-x out of master (and "ours" just has line-x because its 
old), or if line-x was never in master and "ours" really-needed to add it. 
Having merge-base context allows for more accurate conclusions like this:

ours:  has line-x
theirs (master):  does not have line-x
merged:  has line-x
merge-base (older master):  has line-x
conclusion:  I should probably take line-x out of "merged"

ours:  has line-x
theirs (master):  does not have line-x
merged:  has line-x
merge-base:  does not have line-x
conclusion:  I should probably keep line-x in "merged"

Thanks in advance for you feedback.

v/r,
neal 

^ permalink raw reply

* [PATCH] Allow Overriding GIT_BUILD_DIR
From: David A. Greene @ 2012-02-05 22:28 UTC (permalink / raw)
  To: git


Let tests override GIT_BUILD_DIR so git will work if tests are not at
the same directory level as standard git tests.  Prior to this change,
GIT_BUILD_DIR is hardwired to be exactly one directory above where the
test lives.  A test within contrib/, for example, can now use
test-lib.sh and set an appropriate value for GIT_BUILD_DIR.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 t/test-lib.sh |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)


------------------

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a65dfc7..4585138 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -55,6 +55,7 @@ unset $(perl -e '
 		.*_TEST
 		PROVE
 		VALGRIND
+                BUILD_DIR
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
@@ -901,7 +902,14 @@ then
 	# itself.
 	TEST_DIRECTORY=$(pwd)
 fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
+if test -z "$GIT_BUILD_DIR"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/, e.g. for running tests on the test library
+	# itself.
+        GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+fi
 
 if test -n "$valgrind"
 then

--------------------

^ permalink raw reply related

* [PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone
From: Pete Wyckoff @ 2012-02-26  1:06 UTC (permalink / raw)
  To: git; +Cc: Laurent Charrière
In-Reply-To: <1330218385-22309-1-git-send-email-pw@padd.com>

When the --use-client-spec is given to clone, and the clone
path is a subset of the full tree as specified in the client,
future submits will go to the wrong place.

Factor out getClientSpec() so both clone/sync and submit can
use it.  Introduce getClientRoot() that is needed for the client
spec case, and use it instead of p4Where().

Test the five possible submit behaviors (add, modify, rename,
copy, delete).

Reported-by: Laurent Charrière <lcharriere@promptu.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4    |   86 ++++++++++++++++---------
 t/t9809-git-p4-client-view.sh |  142 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 185 insertions(+), 43 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 94f0a12..9ccc87b 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -555,6 +555,46 @@ def p4PathStartsWith(path, prefix):
         return path.lower().startswith(prefix.lower())
     return path.startswith(prefix)
 
+def getClientSpec():
+    """Look at the p4 client spec, create a View() object that contains
+       all the mappings, and return it."""
+
+    specList = p4CmdList("client -o")
+    if len(specList) != 1:
+        die('Output from "client -o" is %d lines, expecting 1' %
+            len(specList))
+
+    # dictionary of all client parameters
+    entry = specList[0]
+
+    # just the keys that start with "View"
+    view_keys = [ k for k in entry.keys() if k.startswith("View") ]
+
+    # hold this new View
+    view = View()
+
+    # append the lines, in order, to the view
+    for view_num in range(len(view_keys)):
+        k = "View%d" % view_num
+        if k not in view_keys:
+            die("Expected view key %s missing" % k)
+        view.append(entry[k])
+
+    return view
+
+def getClientRoot():
+    """Grab the client directory."""
+
+    output = p4CmdList("client -o")
+    if len(output) != 1:
+        die('Output from "client -o" is %d lines, expecting 1' % len(output))
+
+    entry = output[0]
+    if "Root" not in entry:
+        die('Client has no "Root"')
+
+    return entry["Root"]
+
 class Command:
     def __init__(self):
         self.usage = "usage: %prog [options]"
@@ -1119,11 +1159,20 @@ class P4Submit(Command, P4UserMap):
             print "Internal error: cannot locate perforce depot path from existing branches"
             sys.exit(128)
 
-        self.clientPath = p4Where(self.depotPath)
+        self.useClientSpec = False
+        if gitConfig("git-p4.useclientspec", "--bool") == "true":
+            self.useClientSpec = True
+        if self.useClientSpec:
+            self.clientSpecDirs = getClientSpec()
+
+        if self.useClientSpec:
+            # all files are relative to the client spec
+            self.clientPath = getClientRoot()
+        else:
+            self.clientPath = p4Where(self.depotPath)
 
-        if len(self.clientPath) == 0:
-            print "Error: Cannot locate perforce checkout of %s in client view" % self.depotPath
-            sys.exit(128)
+        if self.clientPath == "":
+            die("Error: Cannot locate perforce checkout of %s in client view" % self.depotPath)
 
         print "Perforce checkout for depot path %s located at %s" % (self.depotPath, self.clientPath)
         self.oldWorkingDirectory = os.getcwd()
@@ -2078,33 +2127,6 @@ class P4Sync(Command, P4UserMap):
             print self.gitError.read()
 
 
-    def getClientSpec(self):
-        specList = p4CmdList("client -o")
-        if len(specList) != 1:
-            die('Output from "client -o" is %d lines, expecting 1' %
-                len(specList))
-
-        # dictionary of all client parameters
-        entry = specList[0]
-
-        # just the keys that start with "View"
-        view_keys = [ k for k in entry.keys() if k.startswith("View") ]
-
-        # hold this new View
-        view = View()
-
-        # append the lines, in order, to the view
-        for view_num in range(len(view_keys)):
-            k = "View%d" % view_num
-            if k not in view_keys:
-                die("Expected view key %s missing" % k)
-            view.append(entry[k])
-
-        self.clientSpecDirs = view
-        if self.verbose:
-            for i, m in enumerate(self.clientSpecDirs.mappings):
-                    print "clientSpecDirs %d: %s" % (i, str(m))
-
     def run(self, args):
         self.depotPaths = []
         self.changeRange = ""
@@ -2145,7 +2167,7 @@ class P4Sync(Command, P4UserMap):
             if gitConfig("git-p4.useclientspec", "--bool") == "true":
                 self.useClientSpec = True
         if self.useClientSpec:
-            self.getClientSpec()
+            self.clientSpecDirs = getClientSpec()
 
         # TODO: should always look at previous commits,
         # merge with previous imports, if possible.
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 25e01a4..9642641 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -71,20 +71,24 @@ git_verify() {
 #   - dir2
 #     - file21
 #     - file22
+init_depot() {
+	for d in 1 2 ; do
+		mkdir -p dir$d &&
+		for f in 1 2 ; do
+			echo dir$d/file$d$f >dir$d/file$d$f &&
+			p4 add dir$d/file$d$f &&
+			p4 submit -d "dir$d/file$d$f"
+		done
+	done &&
+	find . -type f ! -name files >files &&
+	check_files_exist dir1/file11 dir1/file12 \
+			  dir2/file21 dir2/file22
+}
+
 test_expect_success 'init depot' '
 	(
 		cd "$cli" &&
-		for d in 1 2 ; do
-			mkdir -p dir$d &&
-			for f in 1 2 ; do
-				echo dir$d/file$d$f >dir$d/file$d$f &&
-				p4 add dir$d/file$d$f &&
-				p4 submit -d "dir$d/file$d$f"
-			done
-		done &&
-		find . -type f ! -name files >files &&
-		check_files_exist dir1/file11 dir1/file12 \
-				  dir2/file21 dir2/file22
+		init_depot
 	)
 '
 
@@ -257,6 +261,122 @@ test_expect_success 'clone --use-client-spec sets useClientSpec' '
 	)
 '
 
+# clone just a subdir of the client spec
+test_expect_success 'subdir clone' '
+	client_view "//depot/... //client/..." &&
+	files="dir1/file11 dir1/file12 dir2/file21 dir2/file22" &&
+	client_verify $files &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	git_verify dir1/file11 dir1/file12
+'
+
+#
+# submit back, see what happens:  five cases
+#
+test_expect_success 'subdir clone, submit modify' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo line >>dir1/file12 &&
+		git add dir1/file12 &&
+		git commit -m dir1/file12 &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file12 &&
+		test_line_count = 2 dir1/file12
+	)
+'
+
+test_expect_success 'subdir clone, submit add' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		echo file13 >dir1/file13 &&
+		git add dir1/file13 &&
+		git commit -m dir1/file13 &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file13
+	)
+'
+
+test_expect_success 'subdir clone, submit delete' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git rm dir1/file12 &&
+		git commit -m "delete dir1/file12" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing dir1/file12
+	)
+'
+
+test_expect_success 'subdir clone, submit copy' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectCopies true &&
+		cp dir1/file11 dir1/file11a &&
+		git add dir1/file11a &&
+		git commit -m "copy to dir1/file11a" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_file dir1/file11a
+	)
+'
+
+test_expect_success 'subdir clone, submit rename' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+	(
+		cd "$git" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.detectRenames true &&
+		git mv dir1/file13 dir1/file13a &&
+		git commit -m "rename dir1/file13 to dir1/file13a" &&
+		"$GITP4" submit
+	) &&
+	(
+		cd "$cli" &&
+		test_path_is_missing dir1/file13 &&
+		test_path_is_file dir1/file13a
+	)
+'
+
+test_expect_success 'reinit depot' '
+	(
+		cd "$cli" &&
+		p4 sync -f &&
+		rm files &&
+		p4 delete */* &&
+		p4 submit -d "delete all files" &&
+		init_depot
+	)
+'
+
 #
 # Rename directories to test quoting in depot-side mappings
 # //depot
-- 
1.7.9.219.g1d56c.dirty

^ permalink raw reply related

* [PATCH 0/2] git-p4: fix submit regression with --use-client-spec
From: Pete Wyckoff @ 2012-02-26  1:06 UTC (permalink / raw)
  To: git; +Cc: Laurent Charrière

This pair of patches fixes a regression that happened with ecb7cf9
(git-p4: rewrite view handling, 2012-01-02).  There are two factors that
affect where files go in the client when submitting:  the cilent spec,
and the depot path.  When the depot path was not exactly the root of
the client, submit fails.

Fix this by always using the true client root.  And also notice that
somebody has to tell the submit path that it should be looking at the
cilent spec.  Save useClientSpec in a configuration variable if it
was specified as an option on the command line.

Junio: can you put this on maint to go out with the next 1.9.x?

Pete Wyckoff (2):
  git-p4: set useClientSpec variable on initial clone
  git-p4: fix submit regression with clientSpec and subdir clone

 Documentation/git-p4.txt      |   10 ++-
 contrib/fast-import/git-p4    |   97 ++++++++++++++++---------
 t/t9809-git-p4-client-view.sh |  159 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 219 insertions(+), 47 deletions(-)

-- 
1.7.9.219.g1d56c.dirty

^ permalink raw reply

* [PATCH 1/2] git-p4: set useClientSpec variable on initial clone
From: Pete Wyckoff @ 2012-02-26  1:06 UTC (permalink / raw)
  To: git; +Cc: Laurent Charrière
In-Reply-To: <1330218385-22309-1-git-send-email-pw@padd.com>

If --use-client-spec was given, set the matching configuration
variable.  This is necessary to ensure that future submits
work properly.

The alternatives of requiring the user to set it, or providing
a command-line option on every submit, are error prone.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt      |   10 +++++++---
 contrib/fast-import/git-p4    |   11 ++++++++++-
 t/t9809-git-p4-client-view.sh |   17 +++++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 78938b2..ed82790 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -303,9 +303,13 @@ CLIENT SPEC
 -----------
 The p4 client specification is maintained with the 'p4 client' command
 and contains among other fields, a View that specifies how the depot
-is mapped into the client repository.  Git-p4 can consult the client
-spec when given the '--use-client-spec' option or useClientSpec
-variable.
+is mapped into the client repository.  The 'clone' and 'sync' commands
+can consult the client spec when given the '--use-client-spec' option or
+when the useClientSpec variable is true.  After 'git p4 clone', the
+useClientSpec variable is automatically set in the repository
+configuration file.  This allows future 'git p4 submit' commands to
+work properly; the submit command looks only at the variable and does
+not have a command-line option.
 
 The full syntax for a p4 view is documented in 'p4 help views'.  Git-p4
 knows only a subset of the view syntax.  It understands multi-line
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..94f0a12 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1428,6 +1428,7 @@ class P4Sync(Command, P4UserMap):
         self.p4BranchesInGit = []
         self.cloneExclude = []
         self.useClientSpec = False
+        self.useClientSpec_from_options = False
         self.clientSpecDirs = None
 
         if gitConfig("git-p4.syncFromOrigin") == "false":
@@ -2136,7 +2137,11 @@ class P4Sync(Command, P4UserMap):
             if not gitBranchExists(self.refPrefix + "HEAD") and self.importIntoRemotes and gitBranchExists(self.branch):
                 system("git symbolic-ref %sHEAD %s" % (self.refPrefix, self.branch))
 
-        if not self.useClientSpec:
+        # accept either the command-line option, or the configuration variable
+        if self.useClientSpec:
+            # will use this after clone to set the variable
+            self.useClientSpec_from_options = True
+        else:
             if gitConfig("git-p4.useclientspec", "--bool") == "true":
                 self.useClientSpec = True
         if self.useClientSpec:
@@ -2455,6 +2460,10 @@ class P4Clone(P4Sync):
             else:
                 print "Could not detect main branch. No checkout/master branch created."
 
+        # auto-set this variable if invoked with --use-client-spec
+        if self.useClientSpec_from_options:
+            system("git config --bool git-p4.useclientspec true")
+
         return True
 
 class P4Branches(Command):
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index c9471d5..25e01a4 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -241,6 +241,23 @@ test_expect_success 'quotes on rhs only' '
 '
 
 #
+# Submit tests
+#
+
+# clone sets variable
+test_expect_success 'clone --use-client-spec sets useClientSpec' '
+	client_view "//depot/... //client/..." &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --use-client-spec --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git config --bool git-p4.useClientSpec >actual &&
+		echo true >true &&
+		test_cmp actual true
+	)
+'
+
+#
 # Rename directories to test quoting in depot-side mappings
 # //depot
 #    - "dir 1"
-- 
1.7.9.219.g1d56c.dirty

^ permalink raw reply related

* Re: [PATCH 2/2] index-pack: reduce memory usage when the pack has large blobs
From: Ian Kumlien @ 2012-02-25 22:45 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8C-8dvXpNTU=JpdupSpS8OuqqTpGvDs6s1ASeKdk9d5Dg@mail.gmail.com>

On Sat, Feb 25, 2012 at 08:49:55AM +0700, Nguyen Thai Ngoc Duy wrote:
> 2012/2/24 Ian Kumlien <pomac@vapor.com>:
> > Writing objects: 100% (1425/1425), 56.06 MiB | 4.62 MiB/s, done.
> > Total 1425 (delta 790), reused 1425 (delta 790)
> > fatal: Out of memory, malloc failed (tried to allocate 3310214315 bytes)
> > fatal: Out of memory, malloc failed (tried to allocate 3310214315 bytes)
> > fatal: Out of memory, malloc failed (tried to allocate 3310214315 bytes)
> > fatal: Out of memory, malloc failed (tried to allocate 3310214315 bytes)
> > To ../test_data/
> >  ! [remote rejected] master -> master (missing necessary objects)
> >  ! [remote rejected] origin/HEAD -> origin/HEAD (missing necessary objects)
> >  ! [remote rejected] origin/master -> origin/master (missing necessary objects)
> > error: failed to push some refs to '../test_data/'
> >
> > So there are additional code paths to look at... =(
> 
> I can't say where that came from. Does this help? (Space damaged, may
> need manual application)

> If not, you might need to apply this to generate coredump, then look
> and see where that failed malloc comes from

Actually, i added a backtrace and used addr2line to confirm my
suspicion... which is:
builtin/index-pack.c:414

ie get_data_from_pack... 

It looks to me like, if we are to support this kind of things, we need a
slightly different approach - instead of passing the data around, it
feels like passing a function pointer around would be beneficial.

Looking at the code i see alot of places where this would be a issue,
just the fact that get_data_from_pack is used in several functions that
might do some small operation and then just free it.

I understand and recognize that my "problem" is not what git was
designed for; it was designed for small files, which is very evident in
how it approaches the data... And I'd most definetly have to look alot
closer to this code... =)

> -- 
> Duy

^ permalink raw reply

* [PATCH v4] Display change history as a diff between two dirs
From: Roland Kaufmann @ 2012-02-25 21:47 UTC (permalink / raw)
  To: gitster; +Cc: git

Watching patches serially it can be difficult to get an overview of how
a pervasive change is distributed through-out different modules. Thus;

Extract snapshots of the files that have changed between two revisions
into temporary directories and launch a graphical tool to show the diff
between them.

Use existing functionality in git-diff to get the files themselves, and
git-difftool to launch the diff viewer.

Based on a script called 'git-diffc' by Nitin Gupta.

Signed-off-by: Roland Kaufmann <rlndkfmn+git@gmail.com>
---
Issue addressed in this revision:
* If changes were made in subdirectories the script though the diff
  set between the two trees was empty, because it only checked for
  regular files.

 contrib/dirdiff/README                 |   10 ++++
 contrib/dirdiff/git-dirdiff--helper.sh |   37 ++++++++++++++++
 contrib/dirdiff/git-dirdiff.sh         |   72 ++++++++++++++++++++++++++++++++
 contrib/dirdiff/git-dirdiff.txt        |   71 +++++++++++++++++++++++++++++++
 4 files changed, 190 insertions(+), 0 deletions(-)
 create mode 100644 contrib/dirdiff/README
 create mode 100755 contrib/dirdiff/git-dirdiff--helper.sh
 create mode 100755 contrib/dirdiff/git-dirdiff.sh
 create mode 100644 contrib/dirdiff/git-dirdiff.txt

diff --git a/contrib/dirdiff/README b/contrib/dirdiff/README
new file mode 100644
index 0000000..d06461a
--- /dev/null
+++ b/contrib/dirdiff/README
@@ -0,0 +1,10 @@
+# install on GNU, BSD:
+for f in "" "--helper"; do
+  b=git-dirdiff$f
+  sudo install -m 0755 contrib/dirdiff/$b.sh $(git --exec-path)/$b
+done
+
+# install on Windows
+for /f %a in ('git --exec-path') do set GIT_PATH=%a
+set GIT_PATH=%GIT_PATH:/=\%
+for %a in ("" "--helper") do copy contrib\dirdiff\git-dirdiff%~a.sh "%GIT_PATH%\%~a" /y
diff --git a/contrib/dirdiff/git-dirdiff--helper.sh b/contrib/dirdiff/git-dirdiff--helper.sh
new file mode 100755
index 0000000..a23efee
--- /dev/null
+++ b/contrib/dirdiff/git-dirdiff--helper.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+#
+# Accumulate files in a changeset into a pre-defined directory.
+#
+# Copyright (C) 2011-2012 Roland Kaufmann
+#
+# Based on a script called git-diffc by Nitin Gupta and valuable
+# suggestions by Junio C. Hamano.
+#
+# This file is licensed under the GPL v2, or a later version
+# at the discretion of the official Git maintainer.
+
+. git-sh-setup
+
+# check that we are called by git-dirdiff
+test -z "$__GIT_DIFF_DIR" &&
+  die Error: Do not call $(basename "$0") directly
+
+# what is the directory name of the file that has changed
+RELDIR=$(dirname "$1") ||
+  exit $?
+
+# don't attempt to copy new or removed files
+if test "$2" != "/dev/null"
+then
+  mkdir -p "$__GIT_DIFF_DIR/old/$RELDIR" ||
+    exit $?
+  cp "$2" "$__GIT_DIFF_DIR/old/$1" ||
+    exit $?
+fi
+if test "$5" != "/dev/null"
+then
+  mkdir -p "$__GIT_DIFF_DIR/new/$RELDIR" ||
+    exit $?
+  cp "$5" "$__GIT_DIFF_DIR/new/$1" ||
+    exit $?
+fi
diff --git a/contrib/dirdiff/git-dirdiff.sh b/contrib/dirdiff/git-dirdiff.sh
new file mode 100755
index 0000000..33c294d
--- /dev/null
+++ b/contrib/dirdiff/git-dirdiff.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Display differences between two commits with a directory comparison.
+#
+# Copyright (C) 2011-2012 Roland Kaufmann
+#
+# Based on a script called git-diffc by Nitin Gupta and valuable
+# suggestions by Junio C. Hamano.
+#
+# This file is licensed under the GPL v2, or a later version
+# at the discretion of the official Git maintainer.
+
+. git-sh-setup
+
+# TMPDIR points to the designated space for temporary files; only if
+# not set use /tmp (MSYS even mounts %TEMP% to there)
+test -z "$TMPDIR" && TMPDIR=/tmp
+
+# create a temporary directory to hold snapshots of changed files
+# fallback on crippled platforms is to use a Perl module
+case $(uname -s) in
+Linux | *BSD | Darwin | windows* | CYGWIN_NT*)
+  __GIT_DIFF_DIR=$(mktemp -d "$TMPDIR/git-dirdiff.XXXXXX")
+  ;;
+*)
+  __GIT_DIFF_DIR=$(perl -e "use File::Temp qw/tempdir/; print tempdir(\"git-dirdiff.XXXXXX\", DIR=>\"$TMPDIR\")")
+  ;;
+esac
+test -d "$__GIT_DIFF_DIR" -a -w "$__GIT_DIFF_DIR" ||
+  die Error: Could not create a temporary subdir in $TMPDIR
+
+# cleanup after we're done
+trap 'rm -rf $__GIT_DIFF_DIR' 0
+
+# export this variable so that scripts called indirectly can access it
+export __GIT_DIFF_DIR
+
+# let the helper script accumulate all changed files into the temporary
+# directory letting 'git diff' do all the heavy lifting
+GIT_EXTERNAL_DIFF=git-dirdiff--helper git --no-pager diff "$@" ||
+  exit $?
+
+# first and second argument will always be the special directory links
+# if there are no hidden files nor regular files, then $3 will be the
+# second wildcard unexpanded
+isempty () {
+  set - $1/.* $1/*
+  test ! -e "$3"
+}
+
+# no-op if no files were changed
+isempty "$__GIT_DIFF_DIR/old" && isempty "$__GIT_DIFF_DIR/new" &&
+  exit 0
+
+# if a different tool is setup for tree comparison, launch that instead
+# if GIT_EXTERNAL_TREEDIFF is not set, then use GIT_EXTERNAL_DIFF
+if test -n "$GIT_EXTERNAL_DIFF"
+then
+  GIT_DIFFTOOL_EXTCMD=$GIT_EXTERNAL_DIFF
+  export GIT_DIFFTOOL_EXTCMD
+fi
+if test -n "$GIT_EXTERNAL_TREEDIFF"
+then
+  GIT_DIFFTOOL_EXTCMD=$GIT_EXTERNAL_TREEDIFF
+  export GIT_DIFFTOOL_EXTCMD
+fi
+
+# run original diff program, reckoning it will understand directories
+# modes and shas does not apply to the root directories so submit dummy
+# values for those, hoping that the diff tool does not use them.
+git-difftool--helper - "$__GIT_DIFF_DIR/old" deadbeef 0755 "$__GIT_DIFF_DIR/new" babeface 0755 ||
+  exit $?
diff --git a/contrib/dirdiff/git-dirdiff.txt b/contrib/dirdiff/git-dirdiff.txt
new file mode 100644
index 0000000..b80430a
--- /dev/null
+++ b/contrib/dirdiff/git-dirdiff.txt
@@ -0,0 +1,71 @@
+git-dirdiff(1)
+==============
+
+NAME
+----
+git-dirdiff - Show changes using directory compare
+
+SYNOPSIS
+--------
+[verse]
+'git dirdiff' [<options>] [<commit> [<commit>]] [--] [<path>...]
+
+DESCRIPTION
+-----------
+'git dirdiff' is a git command that allows you to compare revisions
+as a difference between two directories. 'git dirdiff' is a frontend
+to linkgit:git-diff[1].
+
+OPTIONS
+-------
+See linkgit:git-diff[1] for the list of supported options.
+
+ENVIRONMENT VARIABLES
+---------------------
+'GIT_EXTERNAL_TREEDIFF'::
+	When 'GIT_EXTERNAL_TREEDIFF' is set, the program named by it is
+	used as a diff viewer. The program must accept 7 parameters, where
+	parameter 2 is the path to a temporary directory containing the
+	"old" revision, and parameter 5 is the path of the "new" revision.
+	The other five parameters will only contain dummy values, and their
+	contents are subject to change.
+
+'GIT_EXTERNAL_DIFF'::
+	If and only if 'GIT_EXTERNAL_TREEDIFF' is not set, 'git dirdiff'
+	will use the contents of 'GIT_EXTERNAL_DIFF' as the name of the
+	diff viewer.
+
+CONFIG VARIABLES
+----------------
+If none of the above environment variables are set, 'git dirdiff' will
+use the same config variables as linkgit:git-difftool[1] to determine
+which difftool should be used.
+
+TEMPORARY FILES
+---------------
+'git dirdiff' creates a directory with 'mktemp' to hold snapshots of the
+files which are different in the two revisions. This directory is removed
+when the diff viewer terminates.
+
+NOTES
+-----
+The diff viewer must support being passed directories instead of files
+as its arguments.
++
+Files that are not put under version control are not included when
+viewing the difference between a revision and the working directory.
+
+SEE ALSO
+--------
+linkgit:git-diff[1]::
+	 Show changes between commits, commit and working tree, etc
+
+linkgit:git-difftool[1]::
+	Show changes using common diff tools
+
+linkgit:git-config[1]::
+	 Get and set repository or global options
+
+GIT
+---
+Part of the linkgit:git[1] suite
-- 
1.7.1

^ permalink raw reply related

* [PATCH] rebase -m: only call "notes copy" when rewritten exists and is non-empty
From: Andrew Wong @ 2012-02-25  4:31 UTC (permalink / raw)
  To: git; +Cc: Andrew Wong

This prevents a shell error complaining rebase-merge/rewritten doesn't exist.

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 git-rebase--merge.sh |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 26afc75..5e9d95f 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -90,10 +90,11 @@ call_merge () {
 
 finish_rb_merge () {
 	move_to_original_branch
-	git notes copy --for-rewrite=rebase < "$state_dir"/rewritten
-	if test -x "$GIT_DIR"/hooks/post-rewrite &&
-		test -s "$state_dir"/rewritten; then
-		"$GIT_DIR"/hooks/post-rewrite rebase < "$state_dir"/rewritten
+	if test -s "$state_dir"/rewritten; then
+		git notes copy --for-rewrite=rebase < "$state_dir"/rewritten
+		if test -x "$GIT_DIR"/hooks/post-rewrite; then
+			"$GIT_DIR"/hooks/post-rewrite rebase < "$state_dir"/rewritten
+		fi
 	fi
 	rm -r "$state_dir"
 	say All done.
-- 
1.7.9.2.263.g07763

^ permalink raw reply related

* [PATCH 0/3] parse-options: no- symmetry
From: René Scharfe @ 2012-02-25 19:07 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
	Pierre Habouzit

Boolean long options can be negated by adding a "no-" at the
beginning, unless the flag PARSE_OPT_NONEG is set.  There are
several options defined to start out with "no-" already (e.g.
format-patch --no-numbered).  Their negation with a second
"no-" looks a bit strange.

The flag PARSE_OPT_NEGHELP was introduced to avoid this
awkwardness.  It is used twice (in fast-export and grep) to
define option pairs (--data/--no-data and --index/--no-index)
whose negative part is shown in the help text.

However, PARSE_OPT_NEGHELP is strange as well.  Short options
defined with it do the opposite of what the help text says
(we currently don't have any).  And the multiple negations
are confusing.

This series adds the ability for users to negate options that
already start with "no-" by simply leaving it out.  The last
patch removes PARSE_OPT_NEGHELP as it isn't needed anymore at
that point.

  test-parse-options: convert to OPT_BOOL()
  parse-options: allow positivation of options starting with no-
  parse-options: remove PARSE_OPT_NEGHELP

 Documentation/technical/api-parse-options.txt |    3 +-
 builtin/fast-export.c                         |    4 +-
 builtin/grep.c                                |   15 +++----
 parse-options.c                               |   33 ++++++++------
 parse-options.h                               |    4 --
 t/t0040-parse-options.sh                      |   60 ++++++++++++++++++++++++-
 test-parse-options.c                          |   12 +++--
 7 files changed, 95 insertions(+), 36 deletions(-)

-- 
1.7.9.2

^ permalink raw reply

* [PATCH 1/3] test-parse-options: convert to OPT_BOOL()
From: René Scharfe @ 2012-02-25 19:11 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
	Pierre Habouzit
In-Reply-To: <4F49317A.3080809@lsrfire.ath.cx>

Introduce OPT_BOOL() to test-parse-options and add some tests for
these "true" boolean options. Rename OPT_BOOLEAN to OPT_COUNTUP and
OPTION_BOOLEAN to OPTION_COUNTUP as well.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 t/t0040-parse-options.sh |   60 ++++++++++++++++++++++++++++++++++++++++++++--
 test-parse-options.c     |   12 ++++++----
 2 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index a1e4616..79aefe2 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -10,7 +10,10 @@ test_description='our own option parser'
 cat > expect << EOF
 usage: test-parse-options <options>
 
-    -b, --boolean         get a boolean
+    --yes                 get a boolean
+    -D, --no-doubt        begins with 'no-'
+    -B, --no-fear         be brave
+    -b, --boolean         increment by one
     -4, --or4             bitwise-or boolean with ...0100
     --neg-or4             same as --no-or4
 
@@ -53,6 +56,59 @@ test_expect_success 'test help' '
 
 mv expect expect.err
 
+cat >expect.template <<EOF
+boolean: 0
+integer: 0
+timestamp: 0
+string: (not set)
+abbrev: 7
+verbose: 0
+quiet: no
+dry run: no
+file: (not set)
+EOF
+
+check() {
+	what="$1" &&
+	shift &&
+	expect="$1" &&
+	shift &&
+	sed "s/^$what .*/$what $expect/" <expect.template >expect &&
+	test-parse-options $* >output 2>output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+}
+
+check_unknown() {
+	case "$1" in
+	--*)
+		echo error: unknown option \`${1#--}\' >expect ;;
+	-*)
+		echo error: unknown switch \`${1#-}\' >expect ;;
+	esac &&
+	cat expect.err >>expect &&
+	test_must_fail test-parse-options $* >output 2>output.err &&
+	test ! -s output &&
+	test_cmp expect output.err
+}
+
+test_expect_success 'OPT_BOOL() #1' 'check boolean: 1 --yes'
+test_expect_success 'OPT_BOOL() #2' 'check boolean: 1 --no-doubt'
+test_expect_success 'OPT_BOOL() #3' 'check boolean: 1 -D'
+test_expect_success 'OPT_BOOL() #4' 'check boolean: 1 --no-fear'
+test_expect_success 'OPT_BOOL() #5' 'check boolean: 1 -B'
+
+test_expect_success 'OPT_BOOL() is idempotent #1' 'check boolean: 1 --yes --yes'
+test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
+
+test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
+test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
+
+test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
+test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
+
+test_expect_failure 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
+
 cat > expect << EOF
 boolean: 2
 integer: 1729
@@ -296,7 +352,7 @@ test_expect_success 'OPT_NEGBIT() works' '
 	test_cmp expect output
 '
 
-test_expect_success 'OPT_BOOLEAN() with PARSE_OPT_NODASH works' '
+test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
 	test-parse-options + + + + + + > output 2> output.err &&
 	test ! -s output.err &&
 	test_cmp expect output
diff --git a/test-parse-options.c b/test-parse-options.c
index 36487c4..3c9510a 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -37,7 +37,11 @@ int main(int argc, const char **argv)
 		NULL
 	};
 	struct option options[] = {
-		OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
+		OPT_BOOL(0, "yes", &boolean, "get a boolean"),
+		OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"),
+		{ OPTION_SET_INT, 'B', "no-fear", &boolean, NULL,
+		  "be brave", PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		OPT_COUNTUP('b', "boolean", &boolean, "increment by one"),
 		OPT_BIT('4', "or4", &boolean,
 			"bitwise-or boolean with ...0100", 4),
 		OPT_NEGBIT(0, "neg-or4", &boolean, "same as --no-or4", 4),
@@ -62,11 +66,11 @@ int main(int argc, const char **argv)
 		OPT_ARGUMENT("quux", "means --quux"),
 		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
 			number_callback),
-		{ OPTION_BOOLEAN, '+', NULL, &boolean, NULL, "same as -b",
+		{ OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
-		{ OPTION_BOOLEAN, 0, "ambiguous", &ambiguous, NULL,
+		{ OPTION_COUNTUP, 0, "ambiguous", &ambiguous, NULL,
 		  "positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
-		{ OPTION_BOOLEAN, 0, "no-ambiguous", &ambiguous, NULL,
+		{ OPTION_COUNTUP, 0, "no-ambiguous", &ambiguous, NULL,
 		  "negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
 		OPT_GROUP("Standard options"),
 		OPT__ABBREV(&abbrev),
-- 
1.7.9.2

^ permalink raw reply related

* [PATCH 3/3] parse-options: remove PARSE_OPT_NEGHELP
From: René Scharfe @ 2012-02-25 19:15 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
	Pierre Habouzit
In-Reply-To: <4F49317A.3080809@lsrfire.ath.cx>

PARSE_OPT_NEGHELP is confusing because short options defined with that
flag do the opposite of what the helptext says. It is also not needed
anymore now that options starting with no- can be negated by removing
that prefix. Convert its only two users to OPT_BOOL() and then remove
support for PARSE_OPT_NEGHELP.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin/fast-export.c |    4 +---
 builtin/grep.c        |   15 +++++++--------
 parse-options.c       |    6 ++----
 parse-options.h       |    4 ----
 4 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 08fed98..19509ea 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -647,9 +647,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     "Output full tree for each commit"),
 		OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
 			     "Use the done feature to terminate the stream"),
-		{ OPTION_NEGBIT, 0, "data", &no_data, NULL,
-			"Skip output of blob data",
-			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
+		OPT_BOOL(0, "no-data", &no_data, "Skip output of blob data"),
 		OPT_END()
 	};
 
diff --git a/builtin/grep.c b/builtin/grep.c
index e4ea900..b151467 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -671,7 +671,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct string_list path_list = STRING_LIST_INIT_NODUP;
 	int i;
 	int dummy;
-	int use_index = 1;
+	int no_index = 0;
 	enum {
 		pattern_type_unspecified = 0,
 		pattern_type_bre,
@@ -684,9 +684,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
 			"search in index instead of in the work tree"),
-		{ OPTION_BOOLEAN, 0, "index", &use_index, NULL,
-			"finds in contents not managed by git",
-			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP },
+		OPT_BOOL(0, "no-index", &no_index,
+			 "finds in contents not managed by git"),
 		OPT_BOOLEAN(0, "untracked", &untracked,
 			"search in both tracked and untracked files"),
 		OPT_SET_INT(0, "exclude-standard", &opt_exclude,
@@ -851,7 +850,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		break; /* nothing */
 	}
 
-	if (use_index && !startup_info->have_repository)
+	if (!no_index && !startup_info->have_repository)
 		/* die the same way as if we did it at the beginning */
 		setup_git_directory();
 
@@ -963,11 +962,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!show_in_pager)
 		setup_pager();
 
-	if (!use_index && (untracked || cached))
+	if (no_index && (untracked || cached))
 		die(_("--cached or --untracked cannot be used with --no-index."));
 
-	if (!use_index || untracked) {
-		int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;
+	if (no_index || untracked) {
+		int use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;
 		if (list.nr)
 			die(_("--no-index or --untracked cannot be used with revs."));
 		hit = grep_directory(&opt, &pathspec, use_exclude);
diff --git a/parse-options.c b/parse-options.c
index 8906841..1908996 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -533,7 +533,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 			continue;
 
 		pos = fprintf(outfile, "    ");
-		if (opts->short_name && !(opts->flags & PARSE_OPT_NEGHELP)) {
+		if (opts->short_name) {
 			if (opts->flags & PARSE_OPT_NODASH)
 				pos += fprintf(outfile, "%c", opts->short_name);
 			else
@@ -542,9 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 		if (opts->long_name && opts->short_name)
 			pos += fprintf(outfile, ", ");
 		if (opts->long_name)
-			pos += fprintf(outfile, "--%s%s",
-				(opts->flags & PARSE_OPT_NEGHELP) ?  "no-" : "",
-				opts->long_name);
+			pos += fprintf(outfile, "--%s", opts->long_name);
 		if (opts->type == OPTION_NUMBER)
 			pos += fprintf(outfile, "-NUM");
 
diff --git a/parse-options.h b/parse-options.h
index 2e811dc..def9ced 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,7 +40,6 @@ enum parse_opt_option_flags {
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_NODASH = 32,
 	PARSE_OPT_LITERAL_ARGHELP = 64,
-	PARSE_OPT_NEGHELP = 128,
 	PARSE_OPT_SHELL_EVAL = 256
 };
 
@@ -90,9 +89,6 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
  *   PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
  *				(i.e. '<argh>') in the help message.
  *				Useful for options with multiple parameters.
- *   PARSE_OPT_NEGHELP: says that the long option should always be shown with
- *				the --no prefix in the usage message. Sometimes
- *				useful for users of OPTION_NEGBIT.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK or
-- 
1.7.9.2

^ permalink raw reply related

* [PATCH 2/3] parse-options: allow positivation of options starting, with no-
From: René Scharfe @ 2012-02-25 19:14 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
	Pierre Habouzit
In-Reply-To: <4F49317A.3080809@lsrfire.ath.cx>

Long options can be negated by adding no- right after the leading
two dashes. This is useful e.g. to override options set by aliases.

For options that are defined to start with no- already, this looks
a bit funny. Allow such options to also be negated by removing the
prefix.

The following thirteen options are affected:

	apply          --no-add
	bisect--helper --no-checkout
	checkout-index --no-create
	clone          --no-checkout --no-hardlinks
	commit         --no-verify   --no-post-rewrite
	format-patch   --no-binary
	hash-object    --no-filters
	read-tree      --no-sparse-checkout
	revert         --no-commit
	show-branch    --no-name
	update-ref     --no-deref

The following five are NOT affected because they are defined with
PARSE_OPT_NONEG or the non-negated version is defined as well:

	branch       --no-merged
	format-patch --no-stat             --no-numbered
	update-index --no-assume-unchanged --no-skip-worktree

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/technical/api-parse-options.txt |    3 ++-
 parse-options.c                               |   27 ++++++++++++++++---------
 t/t0040-parse-options.sh                      |    2 +-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 4b92514..2527b7e 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -39,7 +39,8 @@ The parse-options API allows:
 * Short options may be bundled, e.g. `-a -b` can be specified as `-ab`.
 
 * Boolean long options can be 'negated' (or 'unset') by prepending
-  `no-`, e.g. `\--no-abbrev` instead of `\--abbrev`.
+  `no-`, e.g. `\--no-abbrev` instead of `\--abbrev`. Conversely,
+  options that begin with `no-` can be 'negated' by removing it.
 
 * Options and non-option arguments can clearly be separated using the `\--`
   option, e.g. `-a -b \--option \-- \--this-is-a-file` indicates that
diff --git a/parse-options.c b/parse-options.c
index f0098eb..8906841 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -193,13 +193,14 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 		arg_end = arg + strlen(arg);
 
 	for (; options->type != OPTION_END; options++) {
-		const char *rest;
-		int flags = 0;
+		const char *rest, *long_name = options->long_name;
+		int flags = 0, opt_flags = 0;
 
-		if (!options->long_name)
+		if (!long_name)
 			continue;
 
-		rest = skip_prefix(arg, options->long_name);
+again:
+		rest = skip_prefix(arg, long_name);
 		if (options->type == OPTION_ARGUMENT) {
 			if (!rest)
 				continue;
@@ -212,7 +213,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
 		}
 		if (!rest) {
 			/* abbreviated? */
-			if (!strncmp(options->long_name, arg, arg_end - arg)) {
+			if (!strncmp(long_name, arg, arg_end - arg)) {
 is_abbreviated:
 				if (abbrev_option) {
 					/*
@@ -227,7 +228,7 @@ is_abbreviated:
 				if (!(flags & OPT_UNSET) && *arg_end)
 					p->opt = arg_end + 1;
 				abbrev_option = options;
-				abbrev_flags = flags;
+				abbrev_flags = flags ^ opt_flags;
 				continue;
 			}
 			/* negation allowed? */
@@ -239,12 +240,18 @@ is_abbreviated:
 				goto is_abbreviated;
 			}
 			/* negated? */
-			if (strncmp(arg, "no-", 3))
+			if (prefixcmp(arg, "no-")) {
+				if (!prefixcmp(long_name, "no-")) {
+					long_name += 3;
+					opt_flags |= OPT_UNSET;
+					goto again;
+				}
 				continue;
+			}
 			flags |= OPT_UNSET;
-			rest = skip_prefix(arg + 3, options->long_name);
+			rest = skip_prefix(arg + 3, long_name);
 			/* abbreviated and negated? */
-			if (!rest && !prefixcmp(options->long_name, arg + 3))
+			if (!rest && !prefixcmp(long_name, arg + 3))
 				goto is_abbreviated;
 			if (!rest)
 				continue;
@@ -254,7 +261,7 @@ is_abbreviated:
 				continue;
 			p->opt = rest + 1;
 		}
-		return get_value(p, options, flags);
+		return get_value(p, options, flags ^ opt_flags);
 	}
 
 	if (ambiguous_option)
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 79aefe2..aa57299 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -107,7 +107,7 @@ test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
 test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
 test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
 
-test_expect_failure 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
+test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
 
 cat > expect << EOF
 boolean: 2
-- 
1.7.9.2

^ permalink raw reply related

* Re: send-email SMTP/TLS Debugging
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-25 17:55 UTC (permalink / raw)
  To: David A. Greene; +Cc: git
In-Reply-To: <874nufv4ov.fsf@smith.obbligato.org>

On 02/25/2012 06:54 AM, David A. Greene wrote:
> Is there some way to turn on TLS authentication debugging using
> git-send-mail?  I'm trying to send a patch but git (or the mail server,
> I suppose) keeps telling me I have "Incorrect authentication data."
> I've checked the settings in .git/config multiple times and they look
> correct.  How can I debug this further?
Please try 'git send-email --smtp-debug=1 ...'

Zbyszek

^ permalink raw reply


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