Git development
 help / color / mirror / Atom feed
* [PATCH v3 1/4] git-p4: Correct branch base depot path detection
From: Vitor Antunes @ 2011-08-18 23:44 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313711046-23489-1-git-send-email-vitor.hda@gmail.com>

When branch detection is enabled each branch is named in git after their
relative depot path in Perforce. To do this the depot paths are compared against
each other to find their common base path. The current algorithm makes this
comparison on a character by character basis.
Assuming we have the following branches:

//depot/branches/featureA
//depot/branches/featureB

Then the base depot path would be //depot/branches/feature, which is an invalid
depot path.
The current patch fixes this by splitting the path into a list and comparing the
list entries, making it choose correctly //depot/branches as the base path.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4 |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 29a5390..95246e9 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1829,12 +1829,14 @@ class P4Sync(Command, P4UserMap):
                     else:
                         paths = []
                         for (prev, cur) in zip(self.previousDepotPaths, depotPaths):
-                            for i in range(0, min(len(cur), len(prev))):
-                                if cur[i] <> prev[i]:
+                            prev_list = prev.split("/")
+                            cur_list = cur.split("/")
+                            for i in range(0, min(len(cur_list), len(prev_list))):
+                                if cur_list[i] <> prev_list[i]:
                                     i = i - 1
                                     break
 
-                            paths.append (cur[:i + 1])
+                            paths.append ("/".join(cur_list[:i + 1]))
 
                         self.previousDepotPaths = paths
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v3 2/4] git-p4: Allow filtering Perforce branches by user
From: Vitor Antunes @ 2011-08-18 23:44 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313711046-23489-1-git-send-email-vitor.hda@gmail.com>

All branches in the Perforce server are downloaded to allow branch detection. If
you have a centralized server on a remote location and there is a big number of
branches this operation can take some time.
This patch adds the configuration option git-p4.branchUser to allow filtering
the branch list by user. Although this limits the branch maintenance in Perforce
to be done by a single user, it might be an advantage when the number of
branches being used in a specific depot is very small when compared with the
branches available in the server.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4     |    8 +++++++-
 contrib/fast-import/git-p4.txt |    6 ++++++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 95246e9..8b88f97 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1455,7 +1455,13 @@ class P4Sync(Command, P4UserMap):
     def getBranchMapping(self):
         lostAndFoundBranches = set()
 
-        for info in p4CmdList("branches"):
+        user = gitConfig("git-p4.branchUser")
+        if len(user) > 0:
+            command = "branches -u %s" % user
+        else:
+            command = "branches"
+
+        for info in p4CmdList(command):
             details = p4Cmd("branch -o %s" % info["branch"])
             viewIdx = 0
             while details.has_key("View%s" % viewIdx):
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 2ffbccc..97b66b9 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -257,6 +257,12 @@ Perforce server. Will enable --find-copies-harder git argument.
 
   git config [--global] git-p4.detectCopies true
 
+git-p4.branchUser
+
+Only use branch specifications defined by the selected username.
+
+  git config [--global] git-p4.branchUser username
+
 Implementation Details...
 =========================
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v3 0/4] git-p4: Improve branch support
From: Vitor Antunes @ 2011-08-18 23:44 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes

Add missing test case from last version of this set of patches.

Vitor Antunes (4):
  git-p4: Correct branch base depot path detection
  git-p4: Allow filtering Perforce branches by user
  git-p4: Allow branch definition with git config
  git-p4: Add simple test case for branch import

 contrib/fast-import/git-p4     |   40 ++++++++++++++++++++++--
 contrib/fast-import/git-p4.txt |   13 ++++++++
 t/t9800-git-p4.sh              |   64 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 4 deletions(-)

-- 
1.7.5.4

^ permalink raw reply

* Re: [PATCH v2 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Vitor Antunes @ 2011-08-18 23:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Pete Wyckoff, Tor Arvid Lund
In-Reply-To: <7vy5yq5nkb.fsf@alter.siamese.dyndns.org>

On Fri, Aug 19, 2011 at 12:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> This is not a new problem you introduced with this patch, but unless you
> are invoking "git config --bool" in your gitConfig() (I didn't bother to
> check), you will misunderstand different ways to say "Yes", e.g.
>
>        [git-p4]
>                detectRenames
>                detectRenames = on
>                detectRenames = yes
>                detectRenames = 1
>
> If you use --bool, you can rely on the values being either true or false,
> and do not have to do the .lower() thing.
>

Now that I look at this carefully, Tor added the possibility to add arguments to
gitConfig() exactly for that purpose. This is helpful for processing the
detectCopiesHarder option I added.

For detectRenames and detectCopies it is a bit more complex. I think that if I
use --bool or --bool-or-int then it is possible that certain values will fail to
be processed. Let me give you some examples:

        [git-p4]
                detectRenames = true
                detectRenames = 80%
                detectRenames = 80
                detectRenames = 1%
                detectRenames = 1

It will be difficult for me to, for example, to understand if a 1 represents 1%
or true. Or am I overcomplicating this? :)

Thanks,

-- 
Vitor Antunes

^ permalink raw reply

* Re: Missing item in 1.7.7 release notes
From: Junio C Hamano @ 2011-08-18 23:13 UTC (permalink / raw)
  To: Ori Avtalion; +Cc: git
In-Reply-To: <CALgdb5K5iWbqudT_4Euw8yxCmxOz6JVVhFUG=kywjDSTGbx4XA@mail.gmail.com>

The release notes are meant to list only noteworthy updates in a concise
manner. With 100-200 topics graduating to 'master' in each release cycle,
I cannot describe everything that happened in the release and expect the
result to be readable by the target audience, so we need to draw a line
somewhere. Usually miniscule details that affect no user in practice in a
real way are not described (e.g. "fix grammar in an error message").

Interested parties can always look at "git shortlog v1.7.6..v1.7.7".

^ permalink raw reply

* Re: [PATCH v2 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Junio C Hamano @ 2011-08-18 23:04 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git, Pete Wyckoff, Tor Arvid Lund
In-Reply-To: <1313706054-11740-2-git-send-email-vitor.hda@gmail.com>

Vitor Antunes <vitor.hda@gmail.com> writes:

> Copy and rename detection arguments (-C and -M) allow setting a threshold value
> for the similarity ratio. If the similarity is below this threshold the rename
> or copy is ignored and the file is added as new.
> This patch allows setting git-p4.detectRenames and git-p4.detectCopies options
> to an integer value to set the respective threshold.
>
> Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
> ---
>  contrib/fast-import/git-p4 |   18 +++++++++++++-----
>  1 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 6b9de9e..cf719be 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -774,15 +774,23 @@ class P4Submit(Command, P4UserMap):
>  
>          if not self.detectRenames:
>              # If not explicitly set check the config variable
> -            self.detectRenames = gitConfig("git-p4.detectRenames").lower() == "true"
> +            self.detectRenames = gitConfig("git-p4.detectRenames")
>  
> -        if self.detectRenames:
> +        diffOpts = ""
> +        if self.detectRenames.lower() == "true":

This is not a new problem you introduced with this patch, but unless you
are invoking "git config --bool" in your gitConfig() (I didn't bother to
check), you will misunderstand different ways to say "Yes", e.g.

	[git-p4]
                detectRenames
                detectRenames = on
                detectRenames = yes
                detectRenames = 1

If you use --bool, you can rely on the values being either true or false,
and do not have to do the .lower() thing.

^ permalink raw reply

* Re: [PATCH 12/10] support pager.* for external commands
From: Junio C Hamano @ 2011-08-18 22:56 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818220132.GB7799@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Without this patch, any commands that are not builtin would
> not respect pager.* config. For example:
>
>   git config pager.stash false
>   git stash list
>
> would still use a pager.

Unlike the [11/10] patch, I can see why this is a good change.
Thanks.

^ permalink raw reply

* Re: [PATCH 11/10] support pager.* for aliases
From: Junio C Hamano @ 2011-08-18 22:54 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818215909.GA7799@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Until this patch, doing something like:
>
>   git config alias.foo log
>   git config pager.foo /some/specific/pager
>
> would not respect pager.foo at all.

Is it a good thing? Looks too confusing and I am having a hard time to
decide if this is "just because we could" or "because we need to be able
to do this for such and such reasons".

^ permalink raw reply

* Re: update-index --index-info producing spurious submodule commits
From: Junio C Hamano @ 2011-08-18 22:49 UTC (permalink / raw)
  To: Greg Troxel; +Cc: git, Richard Hansen
In-Reply-To: <rmivctuv12s.fsf@fnord.ir.bbn.com>

Greg Troxel <gdt@ir.bbn.com> writes:

> git ls-tree HEAD foo
> git ls-tree HEAD foo | git update-index --index-info

This --index-info definitely looks wrong, if "foo" is a directory, as the
entries in the index are supposed to be either blobs or commits.

As "update-index --index-info" predates "submodule" by a few years or
more, I wouldn't be surprised if the code didn't notice it was fed a wrong
input and produced nonsensical result that happened to be a commit.

The command could just instead barf, saying the input is wrong, but the
option was so low-level that it was deliberately written to accept and
store anything you throw at it --- even when it is nonsensical for the
version of plumbing, later updates to the data structure might have made
it making sense, which was the way to ease development of the system.

By now, we should start enforcing more sanity on its input.

 builtin/update-index.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index a6a23fa..4b32bfe 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -220,6 +220,12 @@ static int process_path(const char *path)
 	return add_one_path(ce, path, len, &st);
 }
 
+static int verify_mode(unsigned int mode)
+{
+	return (mode == 0160000 || mode == 0120000 ||
+		mode == 0100644 || mode == 0100755);
+}
+
 static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 			 const char *path, int stage)
 {
@@ -229,6 +235,9 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 	if (!verify_path(path))
 		return error("Invalid path '%s'", path);
 
+	if (!verify_mode(mode))
+		return error("Invalid mode '%o'", mode);
+
 	len = strlen(path);
 	size = cache_entry_size(len);
 	ce = xcalloc(1, size);

^ permalink raw reply related

* Re: [PATCH 0/10] color and pager improvements
From: Jeff King @ 2011-08-18 22:46 UTC (permalink / raw)
  To: Ingo Brückl; +Cc: git
In-Reply-To: <4e4d94bb.00b9e5c4.bm000@wupperonline.de>

On Fri, Aug 19, 2011 at 12:33:01AM +0200, Ingo Brückl wrote:

> Jeff King wrote on Thu, 18 Aug 2011 14:58:20 -0700:
> 
> > I think Ingo's original complaint was simply that pager.stash didn't
> > actually do anything, not that he wanted some separate config for the
> > various subcommands.
> 
> No, you're wrong.
> 
> My goal was to be able to turn off paging for "stash list" only while all
> other stash commands should continue paging.

Ah, OK. I think the only other stash command that pages is "stash show",
but I don't think it's unreasonable to want paging for that but not for
"list".

I'll take a look at implementing something for that. Though I'll be
traveling for the next few days, so it probably won't be for a bit.

-Peff

^ permalink raw reply

* Re: [PATCH v2] xdiff/xprepare: improve O(n*m) performance in xdl_cleanup_records()
From: Jeff King @ 2011-08-18 22:44 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Git Mailing List, Junio C Hamano, Marat Radchenko
In-Reply-To: <CALUzUxovdmhnRKPaEq01-q0pvez91s_Z_rHWg-FkwcU0VD3HQQ@mail.gmail.com>

On Wed, Aug 17, 2011 at 11:55:32PM +0800, Tay Ray Chuan wrote:

> On Wed, Aug 17, 2011 at 1:21 PM, Jeff King <peff@peff.net> wrote:
> > Wait, what? It was using 0 seconds of user time before, but still taking
> > 8.5 seconds? What was it doing? Did you actually warm up your disk cache
> > before taking these measurements?
> 
> Three runs on the same machine, after a restart.
> 
>   $ time git show >/dev/null
> 
>   real    0m6.505s
>   user    0m0.031s
>   sys     0m0.015s
> [...]

So it is spending only .046s of CPU time, but is taking 6.5 seconds of
wall clock time. Which implies to me that the dataset doesn't fit in
your disk cache, or it is swapping a lot. Or you are on a really
bogged-down multiuser system. :)

But if I understand correctly, your patch is about increasing runtime
performance of a slow algorithm. So is actually the improvement of an
O(m*n) algorithm to an O(n) one, or does your new algorithm have better
memory access patterns that avoid trashing swap?

Downloading the files from the original problem report, I see much
different timings:

  [before]
  $ time git diff >/dev/null
  real    0m7.690s
  user    0m7.648s
  sys     0m0.024s

  [after (your v2 patch)]
  real    0m0.272s
  user    0m0.236s
  sys     0m0.036s

So I think your patch _is_ an improvement in algorithmic runtime. I just
don't see how your numbers make any sense. Am I missing something? Is
msysgit's bash "time" just broken?

-Peff

^ permalink raw reply

* Re: [PATCH 0/10] color and pager improvements
From: Ingo Brückl @ 2011-08-18 22:33 UTC (permalink / raw)
  To: git
In-Reply-To: <20110818215820.GA7767@sigill.intra.peff.net>

Jeff King wrote on Thu, 18 Aug 2011 14:58:20 -0700:

> I think Ingo's original complaint was simply that pager.stash didn't
> actually do anything, not that he wanted some separate config for the
> various subcommands.

No, you're wrong.

My goal was to be able to turn off paging for "stash list" only while all
other stash commands should continue paging.

It is, of course, very usefull to be able to control paging for external
commands and aliases, but in my case I originally wanted to control a
specific subcommand.

Ingo

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2011, #03; Thu, 11)
From: Junio C Hamano @ 2011-08-18 22:34 UTC (permalink / raw)
  To: pascal; +Cc: git
In-Reply-To: <4E4D7DD3.2000701@obry.net>

Pascal Obry <pascal@obry.net> writes:

> Junio,
>
>> * po/cygwin-backslash (2011-08-05) 2 commits
>>   - On Cygwin support both UNIX and DOS style path-names
>>   - git-compat-util: add generic find_last_dir_sep that respects is_dir_sep
>>
>> I think a further refactoring (no, not my suggestion) was offered?
>
> I think the current patchset is fine. It is always possible to improve
> things but the current patch goes in the right direction. So to me it
> is ready as-is.

Not very assuring to hear that only from the original submitter, no?

^ permalink raw reply

* Re: [PATCH RFC] gitk: Allow commit editing
From: Jeff King @ 2011-08-18 22:33 UTC (permalink / raw)
  To: Michal Sojka; +Cc: git, paulus
In-Reply-To: <1313610971-1741-1-git-send-email-sojka@os.inf.tu-dresden.de>

On Wed, Aug 17, 2011 at 09:56:11PM +0200, Michal Sojka wrote:

> Hi, this is a proof of concept patch that allows editing of commits
> from gitk. I often review patches before pushing in gitk and if I
> would like to have an easy way of fixing typos in commit messages etc.
> 
> So the patch adds "Edit this commit" item to gitk's context menu and
> the actual editing is done by non-interactively invoking interactive
> rebase :-) and git gui.

Invoking rebase behind the scenes makes me very nervous. In particular:

  1. There is nothing to indicate to the user that they are rewriting a
     string of commits, which is going to wreak havoc if any of the
     commits have been published elsewhere (either pushed somewhere, or
     even present in another local branch). I.e., rebasing generally
     needs to be a conscious decision of the user.

     Yes, a veteran user who thinks about it will realize there is no
     way to edit an old commit without rebasing, but I suspect relying
     on that is not enough. There should probably be a prominent
     warning at least the first time somebody uses this feature.

  2. Even if you accept the hazard of rewriting commits, you don't pass
     "-p" to rebase. So it will linearize your history. I don't know how
     robust "-p" is these days, and if it's up to the challenge of
     people using it to rebase potentially large segments of complex
     history.

So I think your idea is sane, and if you use it appropriately (by
editing commits in recent-ish linear stretches of history) your patch
works fine. But I really worry that it is going to be a problem for less
clueful users to stumble across in the menu.

-Peff

^ permalink raw reply

* Missing item in 1.7.7 release notes
From: Ori Avtalion @ 2011-08-18 22:32 UTC (permalink / raw)
  To: git

The patch

   d04520e reset: give better reflog messages

is merge into master, but isn't in the release notes.

^ permalink raw reply

* Re: [PATCH 07/10] color: delay auto-color decision until point of use
From: Jeff King @ 2011-08-18 22:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <7vvctu7402.fsf@alter.siamese.dyndns.org>

On Thu, Aug 18, 2011 at 02:59:37PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > diff --git a/color.h b/color.h
> > index a190a25..d715fd5 100644
> > --- a/color.h
> > +++ b/color.h
> > @@ -49,6 +49,16 @@ struct strbuf;
> >  #define GIT_COLOR_NIL "NIL"
> >  
> >  /*
> > + * The first three are chosen to match common usage in the code, and what is
> > + * returned from git_config_colorbool. The "auto" value can be returned from
> > + * config_colorbool, and will be converted by want_color() into either 0 or 1.
> > + */
> > +#define GIT_COLOR_UNKNOWN -1
> > +#define GIT_COLOR_ALWAYS 0
> > +#define GIT_COLOR_NEVER  1
> > +#define GIT_COLOR_AUTO   2
> 
> The ALWAYS/NEVER somehow go against my intuition. Let me trace one
> codepath starting from git_branch_config().
> 
>     branch_use_color is set from git_config_colorbool("color.branch");
>     -> given "never", git_config_colorbool() returns 0;
>     branch_get_color() asks want_color(branch_use_color);
>     -> want_color() returns if the given value is positive.
> 
> Because git_config_colorbool() does not use the above symbolic constants,
> everything goes well, but aren't these two swapped?

Oooops. Yes, they are completely swapped and I'm an idiot. But as you
noticed, we don't actually _use_ them anywhere. I started on replacing
every "0" with NEVER, every "1" with ALWAYS, and every "-1" with
UNKNOWN. But it really bloated the patch, and didn't actually make the
code any more readable.

The only symbolic constant that is really necessary is the AUTO one. I
just felt odd randomly defining "2" as GIT_COLOR_AUTO, but not defining
the other possible values of the enumeration. So definitely they should
be swapped. I'm also fine with just dropping all of them except AUTO.

-Peff

^ permalink raw reply

* Re: [PATCH 0/10] color and pager improvements
From: Junio C Hamano @ 2011-08-18 22:02 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>

Nicely done.

^ permalink raw reply

* Re: [PATCH 07/10] color: delay auto-color decision until point of use
From: Junio C Hamano @ 2011-08-18 21:59 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818050421.GG2889@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> diff --git a/color.h b/color.h
> index a190a25..d715fd5 100644
> --- a/color.h
> +++ b/color.h
> @@ -49,6 +49,16 @@ struct strbuf;
>  #define GIT_COLOR_NIL "NIL"
>  
>  /*
> + * The first three are chosen to match common usage in the code, and what is
> + * returned from git_config_colorbool. The "auto" value can be returned from
> + * config_colorbool, and will be converted by want_color() into either 0 or 1.
> + */
> +#define GIT_COLOR_UNKNOWN -1
> +#define GIT_COLOR_ALWAYS 0
> +#define GIT_COLOR_NEVER  1
> +#define GIT_COLOR_AUTO   2

The ALWAYS/NEVER somehow go against my intuition. Let me trace one
codepath starting from git_branch_config().

    branch_use_color is set from git_config_colorbool("color.branch");
    -> given "never", git_config_colorbool() returns 0;
    branch_get_color() asks want_color(branch_use_color);
    -> want_color() returns if the given value is positive.

Because git_config_colorbool() does not use the above symbolic constants,
everything goes well, but aren't these two swapped?

^ permalink raw reply

* Re: [PATCH 02/10] test-lib: add helper functions for config
From: Junio C Hamano @ 2011-08-18 21:10 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818050114.GB2889@sigill.intra.peff.net>

Thanks.

> ...
> +# Unset a configuration variable, but don't fail if it doesn't exist.
> +test_unconfig () {
> +	git config --unset-all "$@"
> +	config_status=$?
> +	case "$config_status" in
> +	5) # ok, nothing to usnet
> +		config_status=0
> +		;;

Will queue with 's/nothing to usnet/nothing to unset/'.

^ permalink raw reply

* Re: [PATCH 01/10] t7006: modernize calls to unset
From: Junio C Hamano @ 2011-08-18 21:05 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818050044.GA2889@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> These tests break &&-chaining to deal with broken "unset"
> implementations. Instead, they should just use sane_unset.

Thanks. I checked with POSIX again, wondering if I should tone the
"broken" down a bit, but it says:

  Unsetting a variable or function that was not previously set shall not
  be considered an error...

so they deserve "broken" label.

^ permalink raw reply

* [PATCH v2 4/4] git-p4: Add test case for copy detection.
From: Vitor Antunes @ 2011-08-18 22:20 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313706054-11740-1-git-send-email-vitor.hda@gmail.com>

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9800-git-p4.sh |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index d01a1cb..a4f3d66 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -319,6 +319,77 @@ test_expect_success 'detect renames' '
 	rm -rf "$git" && mkdir "$git"
 '
 
+# Copy a file and confirm that copy is not detected in P4.
+# Copy a file with detectCopies option enabled and confirm that copy is not
+# detected in P4.
+# Modify and copy a file with detectCopies option enabled and confirm that copy
+# is detected in P4.
+# Copy a file with detectCopies and detectCopiesHarder options enabled and
+# confirm that copy is detected in P4.
+# Modify and copy a file, configure a big threshold in detectCopies and confirm
+# that copy is not detected in P4.
+# Modify and copy a file, configure a small threshold in detectCopies and
+# confirm that copy is detected in P4.
+test_expect_success 'detect copies' '
+	git init "$git" &&
+	cd "$git" &&
+	cd "$TRASH_DIRECTORY" &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	cd "$git" &&
+	cp file2 file8 &&
+	git add file8 &&
+	git commit -a -m "Copy file2 to file8" &&
+	git diff-tree -r -C HEAD
+	git config git-p4.skipSubmitEditCheck true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file8 &&
+	! p4 filelog //depot/file8 | grep -q "branch from //depot/file" &&
+	cp file2 file9 &&
+	git add file9 &&
+	git commit -a -m "Copy file2 to file9" &&
+	git diff-tree -r -C HEAD
+	git config git-p4.detectCopies true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file9 &&
+	! p4 filelog //depot/file9 | grep -q "branch from //depot/file" &&
+	echo file2 >> file2 &&
+	cp file2 file10 &&
+	git add file2 file10 &&
+	git commit -a -m "Modify and copy file2 to file10" &&
+	git diff-tree -r -C HEAD
+	"$GITP4" submit &&
+	p4 filelog //depot/file10 &&
+	p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+	cp file2 file11 &&
+	git add file11 &&
+	git commit -a -m "Copy file2 to file11" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopiesHarder true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file11 &&
+	p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+	cp file2 file12 &&
+	echo >> file12 &&
+	git add file12 &&
+	git commit -a -m "Copy file2 to file12 with changes" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopies 98 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file12 &&
+	! p4 filelog //depot/file12 | grep -q "branch from //depot/file" &&
+	cp file2 file13 &&
+	echo >> file13 &&
+	git add file13 &&
+	git commit -a -m "Copy file2 to file13 with changes" &&
+	git diff-tree -r -C --find-copies-harder HEAD
+	git config git-p4.detectCopies 80 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file13 &&
+	p4 filelog //depot/file13 | grep -q "branch from //depot/file" &&
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" && mkdir "$git"
+'
+
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&
 	test -n "$pid" &&
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v2 3/4] git-p4: Add test case for rename detection.
From: Vitor Antunes @ 2011-08-18 22:20 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313706054-11740-1-git-send-email-vitor.hda@gmail.com>

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 t/t9800-git-p4.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 97ec975..d01a1cb 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -269,6 +269,56 @@ test_expect_success 'initial import time from top change time' '
 	test $p4time = $gittime
 '
 
+# Rename a file and confirm that rename is not detected in P4.
+# Rename the new file again with detectRenames option enabled and confirm that
+# this is detected in P4.
+# Rename the new file again adding an extra blank line, configure a big
+# threshold in detectRenames and confirm that rename is not detected in P4.
+# Rename the new file again adding another extra blank line, configure a small
+# threshold in detectRenames and confirm that rename is not detected in P4.
+test_expect_success 'detect renames' '
+	git init "$git" &&
+	cd "$git" &&
+	cd "$TRASH_DIRECTORY" &&
+	"$GITP4" clone --dest="$git" //depot@all &&
+	p4 files //depot/* &&
+	cd "$git" &&
+	git mv file1 file4 &&
+	git commit -a -m "Rename file1 to file4" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.skipSubmitEditCheck true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file4 &&
+	! p4 filelog //depot/file4 | grep -q "branch from //depot/file1" &&
+	git mv file4 file5 &&
+	git commit -a -m "Rename file4 to file5" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames true &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file5 &&
+	p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
+	git mv file5 file6 &&
+	echo update >> file6 &&
+	git add file6 &&
+	git commit -a -m "Rename file5 to file6 with changes" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames 95 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file6 &&
+	! p4 filelog //depot/file6 | grep -q "branch from //depot/file5" &&
+	git mv file6 file7 &&
+	echo update >> file7 &&
+	git add file7 &&
+	git commit -a -m "Rename file6 to file7 with changes" &&
+	git diff-tree -r -M HEAD &&
+	git config git-p4.detectRenames 80 &&
+	"$GITP4" submit &&
+	p4 filelog //depot/file7 &&
+	p4 filelog //depot/file7 | grep -q "branch from //depot/file6" &&
+	cd "$TRASH_DIRECTORY" &&
+	rm -rf "$git" && mkdir "$git"
+'
+
 test_expect_success 'shutdown' '
 	pid=`pgrep -f p4d` &&
 	test -n "$pid" &&
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v2 2/4] git-p4: Add description of rename/copy detection options.
From: Vitor Antunes @ 2011-08-18 22:20 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313706054-11740-1-git-send-email-vitor.hda@gmail.com>

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4.txt |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index caa4bb3..2ffbccc 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -232,6 +232,31 @@ git-p4.skipUserNameCheck
 When submitting, git-p4 checks that the git commits are authored by the current
 p4 user, and warns if they are not. This disables the check.
 
+git-p4.detectRenames
+
+Detect renames when submitting changes to Perforce server. Will enable -M git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the rename detection.
+
+  git config [--global] git-p4.detectRenames true
+  git config [--global] git-p4.detectRenames 50
+
+git-p4.detectCopies
+
+Detect copies when submitting changes to Perforce server. Will enable -C git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the copy detection.
+
+  git config [--global] git-p4.detectCopies true
+  git config [--global] git-p4.detectCopies 80
+
+git-p4.detectCopiesHarder
+
+Detect copies even between files that did not change when submitting changes to
+Perforce server. Will enable --find-copies-harder git argument.
+
+  git config [--global] git-p4.detectCopies true
+
 Implementation Details...
 =========================
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v2 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Vitor Antunes @ 2011-08-18 22:20 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313706054-11740-1-git-send-email-vitor.hda@gmail.com>

Copy and rename detection arguments (-C and -M) allow setting a threshold value
for the similarity ratio. If the similarity is below this threshold the rename
or copy is ignored and the file is added as new.
This patch allows setting git-p4.detectRenames and git-p4.detectCopies options
to an integer value to set the respective threshold.

Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
 contrib/fast-import/git-p4 |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 6b9de9e..cf719be 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -774,15 +774,23 @@ class P4Submit(Command, P4UserMap):
 
         if not self.detectRenames:
             # If not explicitly set check the config variable
-            self.detectRenames = gitConfig("git-p4.detectRenames").lower() == "true"
+            self.detectRenames = gitConfig("git-p4.detectRenames")
 
-        if self.detectRenames:
+        diffOpts = ""
+        if self.detectRenames.lower() == "true":
             diffOpts = "-M"
-        else:
-            diffOpts = ""
+        elif self.detectRenames != "":
+            self.detectRenames = int(self.detectRenames)
+            if self.detectRenames >= 0 and self.detectRenames <= 100:
+                diffOpts = "-M%d" % self.detectRenames
 
-        if gitConfig("git-p4.detectCopies").lower() == "true":
+        detectCopies = gitConfig("git-p4.detectCopies")
+        if detectCopies.lower() == "true":
             diffOpts += " -C"
+        elif detectCopies != "":
+            detectCopies = int(detectCopies)
+            if detectCopies >= 0 and detectCopies <= 100:
+                diffOpts += " -C%d" % detectCopies
 
         if gitConfig("git-p4.detectCopiesHarder").lower() == "true":
             diffOpts += " --find-copies-harder"
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH v2 0/4] Support threshold in copy/rename detection
From: Vitor Antunes @ 2011-08-18 22:20 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes

Second version that includes updates recommended by Pete Wyckoff.
Now only "true" and "false" arguments are processed, any other argument
that is no "" is passed directly.

Vitor Antunes (4):
  git-p4: Allow setting rename/copy detection threshold.
  git-p4: Add description of rename/copy detection options.
  git-p4: Add test case for rename detection.
  git-p4: Add test case for copy detection.

 contrib/fast-import/git-p4     |   18 ++++--
 contrib/fast-import/git-p4.txt |   25 ++++++++
 t/t9800-git-p4.sh              |  121 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+), 5 deletions(-)

-- 
1.7.5.4

^ 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